SQL Injection in User List visualization via Admin > Manage #1910

New Issue

2026-10-05T01:00:52+02:00

adam commented

2026-10-05 01:00:52 +02:00

Me and @inesmarcal discover a new vulnerability never before reported in "HOST/admin.php?page=user_list" link, where based on filters should be possible to see the users list, but some of the parameters from these filters are SQL injectable:

Following the request sent through Firefox, we can see that exists several parameters:

If we manipulate the parameters 'max_level' and 'min_register' in order to include a quote and then resend the request, a MYSQL error will appear, which proves the existence of a SQL Injection vulnerability in both fields.

'max_level'

'min_register'

Looking at the source code it's possible to observe that these parameters are grouped in a variable that will use them in the execution of a SQL query:

This could lead to several consequences like "Code Execution" and "Information Disclosure" by manipulate the SQL queries, so these parameters must sanitized before used in any SQL queries. This affect version 12.2.0 and might affect future versions, since there is a reported vulnerability similar to this in version 13.8.0.

PS: please assign this issue to me (@joaosilva21) and my collegue @inesmarcal
PS II: this issue was created in order to have a reference link in the future for the CVE being submitted. When confirmed, the name of this issue will change in order to include the CVE ID in it.

Me and @inesmarcal discover a new vulnerability never before reported in "HOST/admin.php?page=user_list" link, where based on filters should be possible to see the users list, but some of the parameters from these filters are SQL injectable: ![Screenshot_34](https://github.com/user-attachments/assets/69d9e13e-9afe-4d63-b24d-227beddab09d) Following the request sent through Firefox, we can see that exists several parameters: ![Screenshot_35](https://github.com/user-attachments/assets/ffac38bc-1ef7-4035-822f-3f42f83656f2) If we manipulate the parameters 'max_level' and 'min_register' in order to include a quote and then resend the request, a MYSQL error will appear, which proves the existence of a SQL Injection vulnerability in both fields. 'max_level' ![Screenshot_36](https://github.com/user-attachments/assets/bdab34f4-93e9-461f-99c7-d10f7c1f5359) 'min_register' ![Screenshot_37](https://github.com/user-attachments/assets/5706f68f-6c0d-445e-bf64-bb085f2c56ce) Looking at the source code it's possible to observe that these parameters are grouped in a variable that will use them in the execution of a SQL query: ![Screenshot_38](https://github.com/user-attachments/assets/53c6f282-c3bf-4966-a7f4-8a81fa4ee075) ![Screenshot_39](https://github.com/user-attachments/assets/b045accf-e1a0-42cc-b617-8e3ee245f918) This could lead to several consequences like "Code Execution" and "Information Disclosure" by manipulate the SQL queries, so these parameters must sanitized before used in any SQL queries. This affect version 12.2.0 and might affect future versions, since there is a reported vulnerability similar to this in version 13.8.0. PS: please assign this issue to me (@joaosilva21) and my collegue @inesmarcal PS II: this issue was created in order to have a reference link in the future for the CVE being submitted. When confirmed, the name of this issue will change in order to include the CVE ID in it.

adam closed this issue

2026-10-05 01:00:52 +02:00

adam commented

2026-10-05 01:00:53 +02:00

Please admins, assign me to this issue too.

Please admins, assign me to this issue too.

adam commented

2026-10-05 01:00:53 +02:00

OK @inesmarcal :-)

Do you know how we fix this kind of issue in Piwigo? (we add a preg_match)

    if (!empty($params['datetime_parameter']) and !preg_match('/^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d$/', $data['datetime_parameter']))
    {
      return new PwgError(WS_ERR_INVALID_PARAM, 'Invalid input parameter data.datetime_parameter');
    }

OK @inesmarcal :-) Do you know how we fix this kind of issue in Piwigo? (we add a preg_match) ``` if (!empty($params['datetime_parameter']) and !preg_match('/^\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d$/', $data['datetime_parameter'])) { return new PwgError(WS_ERR_INVALID_PARAM, 'Invalid input parameter data.datetime_parameter'); } ```

adam commented

2026-10-05 01:00:54 +02:00

Thank you for the assign! Don't forget to assign @joaosilva21 too!

Relatively to the open issue, we discussed a bit about it and there are 3 options:

do a black list approach, what this means? Ban any inputs in the parameters that may contain some kind of malicious SQL injection payload pattern
do a white list approach, so that you only allow inputs being written in a certain pattern can would not harm the functionality of the queries
change de sql queries to be parameterized

In our opinion the last option should be the one to be implemented since this way the quotes and other inputs will be treat like just regular text, and not something to be concatenated to the currently string that form the sql query. Additionally to this, in certain cases, like parameters that are only numbers, date times, or other that follow specific formats, a white box list could be created to limit inputs that follow to that certain pattern, using the preg_match function.

These are just some of the recommendations that can be implemented (testing firstly if the vulnerability dissappear) :)

Thank you for the assign! Don't forget to assign @joaosilva21 too! Relatively to the open issue, we discussed a bit about it and there are 3 options: - do a black list approach, what this means? Ban any inputs in the parameters that may contain some kind of malicious SQL injection payload pattern - do a white list approach, so that you only allow inputs being written in a certain pattern can would not harm the functionality of the queries - change de sql queries to be parameterized In our opinion the last option should be the one to be implemented since this way the quotes and other inputs will be treat like just regular text, and not something to be concatenated to the currently string that form the sql query. Additionally to this, in certain cases, like parameters that are only numbers, date times, or other that follow specific formats, a white box list could be created to limit inputs that follow to that certain pattern, using the preg_match function. These are just some of the recommendations that can be implemented (testing firstly if the vulnerability dissappear) :)

adam commented

2026-10-05 01:00:55 +02:00

Hi @inesmarcal

As I replied before your question, the answer in Piwigo is :

do a white list approach, so that you only allow inputs being written in a certain pattern can would not harm the functionality of the queries

Yes, the SQL queries with parameters (or "prepared statements") would certainly be the way to go if we rewrote Piwigo from scratch. But changing it now would be such a huge work and risk in term of bugs on stable code. See a related discussion on the forum https://piwigo.org/forum/viewtopic.php?id=30269

Hi @inesmarcal As I replied before your question, the answer in Piwigo is : > do a white list approach, so that you only allow inputs being written in a certain pattern can would not harm the functionality of the queries Yes, the SQL queries with parameters (or "prepared statements") would certainly be the way to go if we rewrote Piwigo from scratch. But changing it now would be such a huge work and risk in term of bugs on stable code. See a related discussion on the forum https://piwigo.org/forum/viewtopic.php?id=30269

adam commented

2026-10-05 01:00:55 +02:00

Hello again!

We read the related discussion and we understood the reasons for why using prepared statements could be a problem, in fact we don't know much too about PHP language. Normally the process for changing from one approach to other would be something not problematic (e.g. in Java, Python, etc), but since here it's not the case I think keeping the white list approach is something good, although as already stated not ideal.
We also thought you can add a little bit of black list approach and try to ban some of the dangerous characters like apostrophes, percentage, quotes and probably some escaped sequence characters (\n, \r, \t, etc). So, in addition to what you have in your regex(a well-defined format), it would be probably interesting to add something that in the presence of the characters that we enumerated (or maybe other ones) exit as invalid, like a 2nd layer of protection. This could be something tangible since the majority of the parameters of Piwigo are alphanumeric, plus maybe hyphen and other minor characters.
If in the future there is some chance I think you should really try to get the prepare statements, since they would introduce a little less of processing time than just checking a regex, that over the years can(or not) get bigger and of course would lead to a more secure environment.

On a final note, please add me to the assignees too x)

Hello again! We read the related discussion and we understood the reasons for why using prepared statements could be a problem, in fact we don't know much too about PHP language. Normally the process for changing from one approach to other would be something not problematic (e.g. in Java, Python, etc), but since here it's not the case I think keeping the white list approach is something good, although as already stated not ideal. We also thought you can add a little bit of black list approach and try to ban some of the dangerous characters like apostrophes, percentage, quotes and probably some escaped sequence characters (\n, \r, \t, etc). So, in addition to what you have in your regex(a well-defined format), it would be probably interesting to add something that in the presence of the characters that we enumerated (or maybe other ones) exit as invalid, like a 2nd layer of protection. This could be something tangible since the majority of the parameters of Piwigo are alphanumeric, plus maybe hyphen and other minor characters. If in the future there is some chance I think you should really try to get the prepare statements, since they would introduce a little less of processing time than just checking a regex, that over the years can(or not) get bigger and of course would lead to a more secure environment. On a final note, please add me to the assignees too x)

adam commented

2026-10-05 01:00:56 +02:00

Considering we received no pull-request for this security issue, I have decided to fix it.

I didn't reproduce the problem with min_level (the user input is already checked, I wonder how you got the SQL injection working)

Considering we received no pull-request for this security issue, I have decided to fix it. I didn't reproduce the problem with `min_level` (the user input is already checked, I wonder how you got the SQL injection working)

adam commented

2026-10-05 01:00:57 +02:00

Heyyy! You didn't reproduce the problem with "min level" because the problem wasn't on there ahah
As I documented above, the problem was on "max_level" parameter

Heyyy! You didn't reproduce the problem with "min level" because the problem wasn't on there ahah As I documented above, the problem was on "max_level" parameter

adam commented

2026-10-05 01:00:57 +02:00

both min_level and max_level are checked, I still wonder how you got the SQL working on this paramater

both min_level and max_level are checked, I still wonder how you got the SQL working on this paramater

Sign in to join this conversation.

master

issue-2560-people

translation

16.x

feat/embedding-based-contextual-tagging

issue-2576-search-include-ai-in-search-results

feat/search-ocr-ai

issue-2391-update-menu-design

15.x

detect-container

issue-2386-update-filters-users-activity

issue-2362-add-filters-options-in-admin

issue-2367-imagemagick7

issue-2225-quicksearch-help

14.x

issue-2164-BatchManagerUnitMode-Redesign

13.x

12.x

issue-1657-SVG_Support

issue-1703-historySearchMethod

11.x

2.10

2.9

2.8

2.7

2.5

2.6

2.4

2.3

2.2

2.1

2.0

1.7

1.3

1.6

1.5

1.4

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: adam/Piwigo#1910