From c809a98dc9519754df18a720c87bb93de6f038ed Mon Sep 17 00:00:00 2001 From: Pierrick Le Gall Date: Mon, 8 May 2023 16:08:51 +0200 Subject: [PATCH] first version, explaining how to prevent SQL injections --- Security-and-coding.md | 59 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Security-and-coding.md diff --git a/Security-and-coding.md b/Security-and-coding.md new file mode 100644 index 0000000..e754a3b --- /dev/null +++ b/Security-and-coding.md @@ -0,0 +1,59 @@ +Piwigo, just like any other web application, is constantly threatened by various kind of attacks. The main ones are SQL injections, (stored) XSS and CSRF. + +## SQL injections + +Maybe the most common attack. The attacker uses a variable in an SQL query to perform what she wants and not what developers had planned. Let's take a simple example. + +``` +$query = ' +SELECT + id, + name, + comment + FROM '.IMAGES_TABLE.' + WHERE id = '.$_GET['image_id'].' +;'; +``` + +Looks right? What if `$_GET['image_id']` (which is a user input from the URL) contains something else than an image id? Something like `1; DELETE FROM users` ? You see how dangerous it can be to use unchecked variables in SQL queries. + +The solution provided by Piwigo is the `check_input_parameter` function: + +```php +function check_input_parameter($param_name, $param_array, $is_array, $pattern, $mandatory=false) +``` + +It's very easy to use. A few examples: + +```php +check_input_parameter('action', $_GET, false, '/^(activate|deactivate|set_default|delete)$/'); // action=activate or action=delete +check_input_parameter('batch', $_GET, false, '/^\d+(,\d+)*$/'); // batch=12,34,56 +check_input_parameter('cat_true', $_POST, true, PATTERN_ID); // cat_true=123 +``` + +Where `PATTERN_ID` is a constant for the most common regular expression to match an identifier (only numeric characters). + +If the user input variable doesn't exist or matches the expected pattern, nothing happens. If the variable doesn't match, the execution immediately stops with a big error. + +Screenshot 2023-05-08 at 15 54 49 + +When to use `check_input_parameter`? On absolutely **every user input when the expected pattern is known**. + +What if we don't have a pattern to check? For example, if you update the title of a photo, you don't have any pattern de match. This time the solution is to "escape" the variable before using it, with the `pwg_db_real_escape_string` function. + +```php +$query = ' +DELETE FROM '.OLD_PERMALINKS_TABLE.' + WHERE permalink=\''.pwg_db_real_escape_string($_GET['delete_permanent']).'\' + LIMIT 1 +;'; +``` + +## (stored) XSS + +TODO explain what it is, when it's a problem and when it's not a problem but a feature + +## CSRF + +TODO explain what a CSRF is +TODO explain how `pwg_token` protects from CSRF attacks \ No newline at end of file