Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVE-2021-3286 - SQL injection incomplete fix #653

Closed
Beuc opened this issue Jan 21, 2021 · 12 comments
Closed

CVE-2021-3286 - SQL injection incomplete fix #653

Beuc opened this issue Jan 21, 2021 · 12 comments
Labels
Stale Automatically marked as stale because it has not had recent activity.

Comments

@Beuc
Copy link

Beuc commented Jan 21, 2021

Describe the bug/issue
Hi, I'm part of the Debian LTS Team and I'm investigating CVE-2020-35545, reported under #629, describes an SQL injection.
The fix from fefb39a / 25c1f89 introduces a black list and a regex to attempt to filter out the malicious payload.
The fix is incomplete and a variation of the example payload escapes the black list.

Have you searched the internet or Github for an answer?
Yes.

To Reproduce

Expected behavior
User input escaped in SQL query.

Desktop

  • OS: Debian GNU/Linux
  • Spotweb Develop
  • PHP 7.0

Additional context
Using a blacklist filter is not recommended in this scenario, because SQL/MySQL queries can be written in varied and unexpected ways, see e.g.:
http://cwe.mitre.org/data/definitions/89.html
https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html
which recommends parametrization and white lists, among other solutions.

The procedure is to request a new CVE identifier when an vulnerability was previously considered fixed.
I plan to do that, unless you tell me not to before the end of the week.

@Beuc Beuc changed the title SQL injection imcomplete fix SQL injection incomplete fix Jan 22, 2021
@Sweepr
Copy link
Collaborator

Sweepr commented Jan 23, 2021

@Beuc

Thank you for bringing this to our attention, we will have a look and see when we can fix this but it may take some time.
If you have any suggestion's on how to fix this, feel welcome to share them with us.

Sweepr added a commit that referenced this issue Jan 23, 2021
Sweepr added a commit that referenced this issue Jan 23, 2021
@Beuc Beuc changed the title SQL injection incomplete fix CVE-2021-3286 - SQL injection incomplete fix Jan 25, 2021
@Beuc
Copy link
Author

Beuc commented Jan 25, 2021

Thank you for bringing this to our attention, we will have a look and see when we can fix this but it may take some time.
If you have any suggestion's on how to fix this, feel welcome to share them with us.

CVE-2021-3286 was assigned to this issue.
My suggestions are in the original comment.

@brianmay
Copy link

Parameterized queries would, combined with a white list instead of a black list, would solve this I think. For information on using parameterized queries with PHP see https://www.php.net/manual/en/pdo.prepared-statements.php

@mesa57
Copy link
Collaborator

mesa57 commented Feb 12, 2021

That's already known for a long time. But the nature of spotweb filters keeps us from using parameterized query's.

@stale
Copy link

stale bot commented Apr 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Automatically marked as stale because it has not had recent activity. label Apr 14, 2021
@carnil
Copy link

carnil commented Apr 15, 2021

It looks though strange that a bot would close the valid issue which is still unfixed. Is there any chance to prevent that (i.e. without need to ping the bug to remove the stable label)

@stale stale bot removed the Stale Automatically marked as stale because it has not had recent activity. label Apr 15, 2021
@Sweepr
Copy link
Collaborator

Sweepr commented Apr 15, 2021

@carnil

You've just prevented that. 😄

@stale
Copy link

stale bot commented Jun 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale Automatically marked as stale because it has not had recent activity. label Jun 14, 2021
@twinden
Copy link

twinden commented Dec 1, 2021

well ping to the bot then.

@mesa57 mesa57 closed this as completed Jan 5, 2022
@Beuc
Copy link
Author

Beuc commented Feb 3, 2022

For clarity, as far as I understand this issue was manually closed not because it was fixed, but because it was deemed unfixable.
Let me know if I'm mistaken.

@mesa57
Copy link
Collaborator

mesa57 commented Feb 3, 2022

You're welcome to fix it if you can.

@Beuc
Copy link
Author

Beuc commented Feb 3, 2022

Thanks for your quick confirmation, appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Automatically marked as stale because it has not had recent activity.
Projects
None yet
Development

No branches or pull requests

6 participants