Announcement

#1 2015-02-09 22:38:22

hamiltont
Member
2015-02-06
12

Filenames not escaped before insertion into SQL Queries

Hey all,

Just ran into an issue where synchronization fails due to a database error if you follow these steps:

1. In local files, update your sync_chars_regex to allow apostrophe in filename
2. Try to synchronize any album containing apostrophe in album name, such as "Visit_to_Mom's"
3. Get SQL error due to unescaped input containing apostrophe

Note: Not sure if the same problem exists for filenames too - this may just happen on album names

Code:

Warning:  [mysql error 1064] You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 's','02 - Visit to Mom's','1','8','8,17','true','true','public','9','5.9')
  , ('' at line 19

INSERT  INTO piwigo_categories
  (id,dir,name,site_id,id_uppercat,uppercats,commentable,visible,status,rank,global_rank)
  VALUES('1','Desktops','Desktops','1',NULL,'1','true','true','public','1','1')
  , ('2','Art','Art','1',NULL,'2','true','true','public','2','2')
  , ('3','Meals','Meals','1',NULL,'3','true','true','public','3','3')
  , ('4','Meals Out','Meals Out','1','3','3,4','true','true','public','1','3.1')
  , ('5','Homemade','Homemade','1','3','3,5','true','true','public','2','3.2')
  , ('6','crepes','crepes','1','5','3,5,6','true','true','public','1','3.2.1')
  , ('7','Funny','Funny','1',NULL,'7','true','true','public','4','4')
  , ('8','2010','2010','1',NULL,'8','true','true','public','5','5')
  , ('9','ORC Boardwalk Building','ORC Boardwalk Building','1' in /var/www/include/dblayer/functions_mysqli.inc.php on line 830

Warning:  [mysql error 1064] You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')
    AND status = 'private'' at line 3

SELECT id
  FROM piwigo_categories
  WHERE id IN ()
    AND status = 'private'
; in /var/www/include/dblayer/functions_mysqli.inc.php on line 830


Fatal error: Call to a member function fetch_assoc() on a non-object in /var/www/include/dblayer/functions_mysqli.inc.php on line 891

If someone would kindly instruct me how to enable getting a stack trace, I'm happy to add one to this so the exact point of failure can be found.

Obviously modifying sync_chars_regex is not a recommended thing to do, but this is still fundamentally a potential vector for SQL injection so it should be patched up.

Piwigo version: 2.7.3
PHP version: 5.4.36-0+deb7u3
MySQL version: 5.7.5-m15

Last edited by hamiltont (2015-02-09 22:41:52)

Offline

 

#2 2015-02-10 09:39:33

plg
Piwigo Team
Nantes, France, Europe
2002-04-05
13822

Re: Filenames not escaped before insertion into SQL Queries

Hi hamiltont,

I don't consider it as a potential SQL injection because this can't be used by "anybody", only by people who have both FTP access and Piwigo admin access. By default apostrophes are forbidden in filenames and we should make obvious that apostrophe is a bad practice.

But anyway, Piwigo have to deal with it, it should be just a few escape before database insert. We already have a bug for it [Bugtracker] ticket 3030

On a recent migration from Menalto, I have locally modified a Piwigo to support the apostrophe (just for the synchronization process, because then I virtualize everything which allows any character in filenames).

Offline

 

#3 2015-02-16 20:50:03

hamiltont
Member
2015-02-06
12

Re: Filenames not escaped before insertion into SQL Queries

Glad to see you're already tracking it!

About this:

> we should make obvious that apostrophe is a bad practice

I understand the codebase is free, so the following feedback comes with no expectations: as a fellow developer, I hope your team comes to view sync_chars_regex as a limitation of piwigo. Piwigo should translate all characters into web-friendly ones upon synchronization. I'm sure there is a long history of why this it, and it's now a large burden to change everything, but the character restrictions are definitely a "con" of piwigo in it's current state. Users shouldn't be expected to understand the difference between web-friendly characters and non-web-friendly ones

Thanks,
Hamilton

Offline

 

Board footer

Powered by FluxBB

github twitter newsletter Donate Piwigo.org © 2002-2024 · Contact