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.

Sign up for GitHub

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

Jump to bottom

Prevent potential Regular expression Denial of Service #3568

Merged
merged 2 commits into from May 15, 2022
Merged

Prevent potential Regular expression Denial of Service #3568

merged 2 commits into from May 15, 2022

Conversation

MarcelBolten
Copy link
Contributor

I don't know if it is possible to write a regex pattern that would trigger a ReDoS but let's be on the safe side and prevent it by escaping all special regular expression characters with preg_quote().

Compare to https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS

@MarcelBolten MarcelBolten marked this pull request as ready for review May 14, 2022 20:42
@NicolasCARPi
Copy link
Contributor

Won't this break some user input searches? (with special characters not intended to be regex) (I just had a quicklook)

@MarcelBolten
Copy link
Contributor Author

This is only relevant for the visibility field and I don't think it will break this field.
The problem with this field is that we allow the user to search for the terms shown in the UI (with all the translations) but in the database we use different terms. In addition, there are the group names which need to be matched to their ids.
But even when there should be a group name with special chars than a regex pattern with these chars escaped will find them.
The real limitations (for all fields) are &, < and > because they get html encoded when used as search input but stored as plain chars or encoded in the db depending on the field.

@MarcelBolten
Copy link
Contributor Author

MarcelBolten commented May 14, 2022 •
edited

Note that for the visibility field the user input is used to build the regex pattern.

NicolasCARPi
NicolasCARPi approved these changes May 15, 2022
View reviewed changes
Copy link
Contributor

@NicolasCARPi NicolasCARPi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok then!

@NicolasCARPi NicolasCARPi merged commit e48d4ce into hypernext May 15, 2022
@NicolasCARPi NicolasCARPi deleted the search branch May 15, 2022 07:22
NicolasCARPi added a commit that referenced this pull request May 18, 2022
@NicolasCARPi
Merge branch 'hypernext' into demo
ed7277e 
* hypernext:
  fix images in pdf and mixed files storage in zip
  modernize and fix annotate image action
  fix h1 was same size as h2
  drop the use_ove user setting (#3572)
  filter out archived users from user list for autocompletion in admin
  fix select elements in show mode menu
  add more mathjax-ignore in show mode
  add mathjax-ignore class to tags to prevent mathjax render
  Prevent potential Regular expression Denial of Service (#3568)
  remove clickable class from elements with a data-action
  add optgroup to author/group select on search page
  yarn upgrade
  make all status timestampable (#3567)
  Fix search (#3566)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NicolasCARPi NicolasCARPi NicolasCARPi approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MarcelBolten @NicolasCARPi