adam/Piwigo
1
0 Fork 0
mirror of https://github.com/Piwigo/Piwigo.git synced 2026-10-30 21:52:00 +01:00
Code Issues 657 Packages Projects Releases Wiki Activity

[12.x] synchronisation, quote not supported in album name #1380

New Issue
Open
opened 2026-10-05 00:41:27 +02:00 by adam · 17 comments
adam commented 2026-10-05 00:41:27 +02:00
Owner
Copy link
Copy source

I'm try to create an album with the name

Vacances d'été à la montagne

And it seems the quote breaks the request now. I have some album with quote created with piwigo 11.
I'm using mysqli.

Here the error I see on the page:



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 'été à la montagne','0','16.0','true','true','private','689')' at line 3

INSERT  INTO `piwigo_categories`
  (`name`,`rank`,`global_rank`,`commentable`,`visible`,`status`,`id_uppercat`)
  VALUES('Vacances d'été à la montagne','0','16.0','true','true','private','689') in /app/include/dblayer/functions_mysqli.inc.php on line 847



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 ')
    AND status = 'private'' at line 3

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


Fatal error: Uncaught Error: Call to a member function fetch_assoc() on bool in /app/include/dblayer/functions_mysqli.inc.php:908 Stack trace:
#0 /app/admin/include/functions.php(2879): query2array()
#1 /app/admin/include/functions.php(1583): add_permission_on_category()
#2 /app/admin/cat_list.php(163): create_virtual_category()
#3 /app/admin.php(314): include('...')
#4 {main} thrown in /app/include/dblayer/functions_mysqli.inc.php on line 908

As far as I can see, the parameters that come from the browser are injected like that in the query with no protection.
I think the best would be to use binding when doing query, this is dangerous !

I'm try to create an album with the name ``` Vacances d'été à la montagne ``` And it seems the quote breaks the request now. I have some album with quote created with piwigo 11. I'm using mysqli. Here the error I see on the page: ``` 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 'été à la montagne','0','16.0','true','true','private','689')' at line 3 INSERT INTO `piwigo_categories` (`name`,`rank`,`global_rank`,`commentable`,`visible`,`status`,`id_uppercat`) VALUES('Vacances d'été à la montagne','0','16.0','true','true','private','689') in /app/include/dblayer/functions_mysqli.inc.php on line 847 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 ') AND status = 'private'' at line 3 SELECT id FROM piwigo_categories WHERE id IN () AND status = 'private' ; in /app/include/dblayer/functions_mysqli.inc.php on line 847 Fatal error: Uncaught Error: Call to a member function fetch_assoc() on bool in /app/include/dblayer/functions_mysqli.inc.php:908 Stack trace: #0 /app/admin/include/functions.php(2879): query2array() #1 /app/admin/include/functions.php(1583): add_permission_on_category() #2 /app/admin/cat_list.php(163): create_virtual_category() #3 /app/admin.php(314): include('...') #4 {main} thrown in /app/include/dblayer/functions_mysqli.inc.php on line 908 ``` As far as I can see, the parameters that come from the browser are injected like that in the query with no protection. I think the best would be to use binding when doing query, this is dangerous ! <!-- github-sync-id:1063876461 -->
adam commented 2026-10-05 00:41:28 +02:00
Author
Owner
Copy link
Copy source

Same here still, Fedora 35

Very frustrating, as no errors, just the green "created" bar points at id 0

Piwigo 12.2.0
Operating system: Linux
PHP: 8.0.16
MySQL: 10.5.13-MariaDB
Graphics Library: External ImageMagick 6.9.12-37

Same here still, Fedora 35 Very frustrating, as no errors, just the green "created" bar points at id 0 Piwigo 12.2.0 Operating system: Linux PHP: 8.0.16 MySQL: 10.5.13-MariaDB Graphics Library: External ImageMagick 6.9.12-37 <!-- github-comment-id:1059747955 -->
adam commented 2026-10-05 00:41:29 +02:00
Author
Owner
Copy link
Copy source

Hi @celogeek
try changing your album name to "Vacance"
it will give a search lead, if this is concluded there will be a code formula to add in localfiles editor hence the regex.
thanks

Hi @celogeek try changing your album name to "Vacance" it will give a search lead, if this is concluded there will be a code formula to add in localfiles editor hence the regex. thanks <!-- github-comment-id:1059793572 -->
adam commented 2026-10-05 00:41:30 +02:00
Author
Owner
Copy link
Copy source

Well, this works:

Vacances d\'été à la montagne

It really looks like SQL injections is possible.

Well, this works: ``` Vacances d\'été à la montagne ``` It really looks like SQL injections is possible. <!-- github-comment-id:1059905870 -->
adam commented 2026-10-05 00:41:31 +02:00
Author
Owner
Copy link
Copy source

Oh dear.

Unless I miss something, cat_list.php passed the POST of the name direct. Maybe the parent id too

elseif (isset($_POST['submitAdd']))
{
  $output_create = create_virtual_category(
    $_POST['virtual_name'],
    @$_GET['parent_id']
  );

functions.php then does totally the wrong thing.

 WHERE id_uppercat '.(empty($parent_id) ? 'IS NULL' : '= '.$parent_id).'

multiple other occurrences, simple SQL injection.

I've contacted Piwigo via the page on their web site. I can't see a way to tag this issue as being sensitive and hide it.

At least it requires authorised user (I hope, not checked)...

Oh dear. Unless I miss something, cat_list.php passed the POST of the name direct. Maybe the parent id too ``` elseif (isset($_POST['submitAdd'])) { $output_create = create_virtual_category( $_POST['virtual_name'], @$_GET['parent_id'] ); ``` functions.php then does totally the wrong thing. ``` WHERE id_uppercat '.(empty($parent_id) ? 'IS NULL' : '= '.$parent_id).' ``` multiple other occurrences, simple SQL injection. I've contacted Piwigo via the page on their web site. I can't see a way to tag this issue as being sensitive and hide it. At least it requires authorised user (I hope, not checked)... <!-- github-comment-id:1059946580 -->
adam commented 2026-10-05 00:41:32 +02:00
Author
Owner
Copy link
Copy source

hello
try in "localfiles editor"
for example ==> $conf['sync_chars_regex'] ='/^[a-zA-Z0-9-_.é ]+$/;

hello try in "localfiles editor" for example ==> $conf['sync_chars_regex'] ='/^[a-zA-Z0-9-_.é ]+$/; <!-- github-comment-id:1059950587 -->
adam commented 2026-10-05 00:41:32 +02:00
Author
Owner
Copy link
Copy source

@MatthieuLP @plegall #security

@MatthieuLP @plegall #security <!-- github-comment-id:1059950852 -->
adam commented 2026-10-05 00:41:33 +02:00
Author
Owner
Copy link
Copy source

It might be able to "fix" this with addslashes in the right place... Seems to be used very sparingly in the current code however.

Would the project be open to a patch that added this ?

It might be able to "fix" this with addslashes in the right place... Seems to be used very sparingly in the current code however. Would the project be open to a patch that added this ? <!-- github-comment-id:1059953431 -->
adam commented 2026-10-05 00:41:34 +02:00
Author
Owner
Copy link
Copy source

@tomchiverton in admin/cat_list.php did you see this line:

check_input_parameter('parent_id', $_GET, false, PATTERN_ID);

Maybe you did, but certainly you don't know what happens if $_GET['parent_id'] contains anything else than digits :-) It would immediately fail. It would never go into the SQL query.

Security matters to us. It's not because it's not done "the way you expect it to be" that it's not done another way :-) So unless you have a case where a faulty $_GET['parent_id'] breaks a SQL query, we can consider this as a non-problem.

@tomchiverton in `admin/cat_list.php` did you see this line: ``` check_input_parameter('parent_id', $_GET, false, PATTERN_ID); ``` Maybe you did, but certainly you don't know what happens if $_GET['parent_id'] contains anything else than digits :-) It would immediately fail. It would never go into the SQL query. Security matters to us. It's not because it's not done "the way you expect it to be" that it's not done another way :-) So unless you have a case where a faulty `$_GET['parent_id']` breaks a SQL query, we can consider this as a non-problem. <!-- github-comment-id:1060739168 -->
adam commented 2026-10-05 00:41:35 +02:00
Author
Owner
Copy link
Copy source

I've changed the title of this issue. It's not an SQL injection per say. It's only a “problem" when adding albums by synchronisation (based on filesystem). I personally think it's a really bad idea to use quotes, spaces, mixing small and capital letters... in file names. You can do it. Your filesystem will let you do it. It remains a bad idea. Who has tried to use "/" character in a filename on Linux?

In Piwigo, when you use the synchronisation system to add albums/photos, there is the "physical" (files in the filesystem) layer and the "logical" (database) layer. You can prefectly set the title of your album to "what'ever You Want 专辑" once added in your Piwigo, because it's only in the database, not on the filesystem.

I've changed the title of this issue. It's not an SQL injection per say. It's only a “problem" when adding albums by synchronisation (based on filesystem). I personally think it's a really bad idea to use quotes, spaces, mixing small and capital letters... in file names. You can do it. Your filesystem will let you do it. It remains a bad idea. Who has tried to use "/" character in a filename on Linux? In Piwigo, when you use the synchronisation system to add albums/photos, there is the "physical" (files in the filesystem) layer and the "logical" (database) layer. You can prefectly set the title of your album to "what'ever You Want 专辑" once added in your Piwigo, because it's only in the database, not on the filesystem. <!-- github-comment-id:1060759343 -->
adam commented 2026-10-05 00:41:35 +02:00
Author
Owner
Copy link
Copy source

I use the interface in Admin > Albums > Manage
I click on the button "Create", then enter this title.
I've got an error.

When I add a backslash, it works.
So nothing related to the local system, it is only on the Category in the database.

I use the interface in Admin > Albums > Manage I click on the button "Create", then enter this title. I've got an error. When I add a backslash, it works. So nothing related to the local system, it is only on the Category in the database. <!-- github-comment-id:1060953029 -->
adam commented 2026-10-05 00:41:36 +02:00
Author
Owner
Copy link
Copy source

By the way I've made a tools to sync a full struct with album name, and I temporary add the backslash on the parameters Category Name, until the SQL issue is solve.

We can see the SQL with the string replaced directly in the error message. So it is a SQL injection issue.

Usually it is highly recommanded to use the binding feature of the SQL drivers to avoid any possible sql injection.

What if I use a name like this directly in the interface:

Vacances'; DROP DATABASE photos; '-- '

I won't try, but I'm sure it is possible to drop a table or the database just by trying a name like that.

By the way I've made a tools to sync a full struct with album name, and I temporary add the backslash on the parameters Category Name, until the SQL issue is solve. We can see the SQL with the string replaced directly in the error message. So it is a SQL injection issue. Usually it is highly recommanded to use the binding feature of the SQL drivers to avoid any possible sql injection. What if I use a name like this directly in the interface: ``` Vacances'; DROP DATABASE photos; '-- ' ``` I won't try, but I'm sure it is possible to drop a table or the database just by trying a name like that. <!-- github-comment-id:1060956824 -->
adam commented 2026-10-05 00:41:37 +02:00
Author
Owner
Copy link
Copy source
INSERT  INTO `piwigo_categories`
  (`name`,`rank`,`global_rank`,`commentable`,`visible`,`status`,`id_uppercat`)
  VALUES('Vacances d'été à la montagne'

We can clearly see here what it try to do (just build the SQL string with the input from the browser)

``` INSERT INTO `piwigo_categories` (`name`,`rank`,`global_rank`,`commentable`,`visible`,`status`,`id_uppercat`) VALUES('Vacances d'été à la montagne' ``` We can clearly see here what it try to do (just build the SQL string with the input from the browser) <!-- github-comment-id:1060957944 -->
adam commented 2026-10-05 00:41:38 +02:00
Author
Owner
Copy link
Copy source

I created a test table, then tried to create a new album called

Vacances'; DROP table newTable; '-- '

but the new table was not removed

Some command line fiddling e.g. ...virtual_name=Vacances'; DROP table newTable; '... didn't yield an exploit either.

I created a test table, then tried to create a new album called Vacances'; DROP table newTable; '-- ' but the new table was not removed Some command line fiddling e.g. `...virtual_name=Vacances'; DROP table newTable; '...` didn't yield an exploit either. <!-- github-comment-id:1061056756 -->
adam commented 2026-10-05 00:41:38 +02:00
Author
Owner
Copy link
Copy source

single_insert(CATEGORIES_TABLE, $insert); appears to protect the virtual_name parameter but I'm not sure how or where this happens to test it fully

` single_insert(CATEGORIES_TABLE, $insert); ` appears to protect the virtual_name parameter but I'm not sure how or where this happens to test it fully <!-- github-comment-id:1061063835 -->
adam commented 2026-10-05 00:41:39 +02:00
Author
Owner
Copy link
Copy source

We can see the SQL with the string replaced directly in the error message. So it is a SQL injection issue.

How did you do that ? Here when I try to create an album in the admin with a ' in the name, it just refreshes the page and has a green banner saying "Album added Edit album" but the edit link is ..../admin.php?page=album-0

My versions are above.

> We can see the SQL with the string replaced directly in the error message. So it is a SQL injection issue. How did you do that ? Here when I try to create an album in the admin with a ' in the name, it just refreshes the page and has a green banner saying "Album added _Edit album_" but the edit link is ..../admin.php?page=album-0 My versions are above. <!-- github-comment-id:1061079294 -->
adam commented 2026-10-05 00:41:40 +02:00
Author
Owner
Copy link
Copy source

I use the interface in Admin > Albums > Manage I click on the button "Create", then enter this title. I've got an error.

No problem for me.
Screenshot_2022-03-08 galerie photo de demolejeune Administration de Piwigo
Screenshot_2022-03-08 galerie photo de demolejeune Administration de Piwigo(1)

The difference I see between you and me is that you're on PHP 8 while I'm on PHP 7. We're going to make some test specifically on PHP 8. Could be related to magic_quotes being removed from PHP 8.

> I use the interface in Admin > Albums > Manage I click on the button "Create", then enter this title. I've got an error. No problem for me. ![Screenshot_2022-03-08 galerie photo de demolejeune Administration de Piwigo](https://user-images.githubusercontent.com/9326959/157186970-e6a92254-0cf2-4343-9535-d30e5aa560a1.png) ![Screenshot_2022-03-08 galerie photo de demolejeune Administration de Piwigo(1)](https://user-images.githubusercontent.com/9326959/157186978-5b7e2ade-a8f7-4eea-8cad-bb8feee11305.png) The difference I see between you and me is that you're on PHP 8 while I'm on PHP 7. We're going to make some test specifically on PHP 8. Could be related to `magic_quotes` being removed from PHP 8. <!-- github-comment-id:1061484141 -->
adam commented 2026-10-05 00:41:41 +02:00
Author
Owner
Copy link
Copy source

After testing on PHP8 environment, we find that creating albums with a quote breaks the page.
We'll work on a fix for PHP8 users.

After testing on PHP8 environment, we find that creating albums with a quote breaks the page. We'll work on a fix for PHP8 users. <!-- github-comment-id:1061603833 -->
Sign in to join this conversation.
No Branch/Tag Specified
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
adam (Adam Wojcieszonek)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: adam/Piwigo#1380