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

Fix possible SQL injection in admin page #1340

Merged
merged 1 commit into from Jan 13, 2023

Conversation

madsi1m
Copy link
Contributor

@madsi1m madsi1m commented Jan 12, 2023

Sanitise email field from admin form and pass it through as a placeholder so PDO uses prepare function + improved UX and search emails in lowercase.

…lder so PDO uses prepare function + improved UX and search emails in lowercase
@github-actions
Copy link

If there are selenium UI results for this code they will be at filesenderuici@93e55df

@monkeyiq monkeyiq merged commit c86316b into development Jan 13, 2023
@madsi1m madsi1m deleted the admin_transfers_improvements branch January 13, 2023 02:09
@liedekef
Copy link
Contributor

Hum ... what I see here is not a change to a possible SQL injection at all. If the only concern is to allow an email to have "%" before or after it to allow wildcard searches:

  • check if it starts with a "%" and set a boolean var
  • check if it ends with a "%" and set another boolean var
  • sanitize the email
  • re-add the beginning and/or ending "%" to the sanitized value based on the boolean vars

The way it is now coded it seems easy to abuse an admin session in place by sending a link with as get-param a very "bad" email ... While PDO prepare is difficult to hack, it is not impossible:
https://stackoverflow.com/questions/134099/are-pdo-prepared-statements-sufficient-to-prevent-sql-injection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants