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

ascanrulesBeta: Address ReDoS in Insecure HTTP Methods rule #6094

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ascanrulesBeta: Address ReDoS in Insecure HTTP Methods rule #6094

wants to merge 1 commit into from

Conversation

kingthorin
Copy link
Member

@kingthorin kingthorin commented Jan 13, 2025 •
edited
Loading

Overview

  • CHANGELOG > Add fix note.
  • InsecureHttpMethodScanRule > Adjust regex pattern for Google title elements. There's no reason to look for unlimited length character strings. It is doubtful that google would produce content that might cause a ReDoS, but limiting the regex is "safest".

Related Issues

https://github.com/zaproxy/zap-extensions/security/code-scanning/89

Checklist

  • Update help
  • Update changelog
  • Run ./gradlew spotlessApply for code formatting
  • Write tests
  • Check code coverage
  • Sign-off commits
  • Squash commits
  • Use a descriptive title

@psiinon
Copy link
Member

psiinon commented Jan 13, 2025 •
edited
Loading

Logo
Checkmarx One – Scan Summary & Details – b25975fb-d629-4695-8112-f08377f1e791

Great job, no security vulnerabilities found in this Pull Request

@thc202
Copy link
Member

thc202 commented Jan 15, 2025 •
edited
Loading

Needs rebase, worth adding a test (if straightforward).

@kingthorin
ascanrulesBeta: Address ReDoS in Insecure HTTP Methods rule
3a7ec35 
- CHANGELOG > Add fix note.
- InsecureHttpMethodScanRule > Adjust regex pattern for Google title
elements. There's no reason to look for unlimited length character
strings. It is doubtful that google would produce content that might
cause a ReDoS, but limiting the regex is "safest".

Signed-off-by: kingthorin <[email protected]>
@kingthorin
Copy link
Member Author

Rebased.

I worked quickly on a Unit Test, I couldn't get it to delay consistently/significantly even inserting 5million characters 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kingthorin @psiinon @thc202