Piwigo / piwigo-openstreetmap

OpenStreetMap integration for Piwigo
http://piwigo.org/ext/extension_view.php?eid=701
GNU General Public License v3.0
35 stars 35 forks source link

SQL injection in tag module #163

Closed lkew closed 11 months ago

lkew commented 6 years ago

Hello, In the tag section, it's not escaping quotes; making it vulnerable to SQL injection: https:///admin.php?page=plugin&section=piwigo-openstreetmap%2Fadmin%2Fadmin.php&tab=tag

Result:

Warning: [mysql error 1064] You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'Ernz'' at line 3

SELECT id FROM g_tags WHERE name = 'city:Vallée de l'Ernz' ; in //include/dblayer/functions_mysqli.inc.php on line 845

I think I could fix it; but it would probably break all coding rules of both the plugin and Piwigo; so I thought I'd rather report it. If you need anything additional, please let me know.

Thanks, Tristan

xbgmsharp commented 6 years ago

Hello,

The SQL query just need to be escape. It is not an SQL injection. however it needs to be fixed. Could you please details how to reproduce the issue? I could not find any similar SQL query in the code. https://github.com/xbgmsharp/piwigo-openstreetmap/blob/master/admin/admin_tag.php

Wernfried commented 2 years ago

Code like

    if ( $sync_options['cat_id']!=0 )
    {
        $query=' SELECT id FROM '.CATEGORIES_TABLE.' WHERE ';

        if ( $sync_options['subcats_included'])
            $query .= 'uppercats REGEXP \'(^|,)'.$sync_options['cat_id'].'(,|$)\'';
        else
            $query .= 'id='.$sync_options['cat_id'];
            $cat_ids = array_from_query($query, 'id');

        $query='SELECT `id`, `name`, `latitude`, `longitude` FROM '.IMAGES_TABLE.' INNER JOIN '.IMAGE_CATEGORY_TABLE.' ON id=image_id
            WHERE '. SQL_EXIF .' AND category_id IN ('.implode(',', $cat_ids).')
            GROUP BY id';
    }
    else
    {
        $query='SELECT `id`, `name`, `latitude`, `longitude` FROM '.IMAGES_TABLE.' WHERE '. SQL_EXIF;
    }

is a prime example for potential SQL injection. Did you never hear about Prepared Statements?

xbgmsharp commented 2 years ago

Feel free to provide a PR to fix it.

fgtham commented 1 year ago

This is still an issue with the current version. Here's how to reproduce:

  1. Assign the following coordinates to an image: 43.377210 lat, 6.721320 lon
  2. In the plugin's keyword tag, tick 'County' and 'State'.
  3. A simulation run shows that it will insert a tag "Provence-Alpes-Côte d'Azur".
  4. Untick 'Simulation' and execute the plugin to trigger the error.
fgtham commented 1 year ago

Here's a fix for the issue:

--- a/admin/include/functions.php    2023-09-30 12:03:25.072790400 +0200
+++ b/admin/include/functions.php    2023-09-30 21:08:06.881713481 +0200
@@ -1720,7 +1720,7 @@
   $query = '
 SELECT id
   FROM '.TAGS_TABLE.'
-  WHERE name = \''.$tag_name.'\'
+  WHERE name = \''.pwg_db_real_escape_string($tag_name).'\'
 ;';
   if (count($existing_tags = query2array($query, null, 'id')) == 0)
   {
xbgmsharp commented 1 year ago

Thanks. Could you make this a PR? Also is admin/include/functions.php part of the plugin or Piwigo?

fgtham commented 1 year ago

It's part of piwigo, not the plugin. I will open a PR over there.

fgtham commented 11 months ago

The problem can be fixed in piwigo-openstreetmap, see #237.