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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Fix for Regular Expression Denial of Service (ReDoS) - huntr.dev #74

Closed
wants to merge 2 commits into from

Conversation

huntr-helper
Copy link

https://huntr.dev/users/mufeedvh has fixed the Regular Expression Denial of Service (ReDoS) vulnerability 馃敤. mufeedvh has been awarded $25 for fixing the vulnerability through the huntr bug bounty program 馃挼. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A
Version Affected | ALL
Bug Fix | YES
Original Pull Request | 418sec#1
GitHub Issue | #70
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/maven/url-regex/1/README.md

User Comments:

馃搳 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-maven-url-regex

鈿欙笍 Description *

The project url-regex was validating URLs with a regex vulnerable to ReDoS (Regex Denial of Service) with RegExp().

馃捇 Technical Description *

The implemented Regex patterns to validate URLs are vulnerable to ReDoS:

const protocol = `(?:(?:[a-z]+:)?//)${options.strict ? '' : '?'}`;
const auth = '(?:\\S+(?::\\S*)?@)?';
const ip = ipRegex.v4().source;
const host = '(?:(?:[a-z\\u00a1-\\uffff0-9][-_]*)*[a-z\\u00a1-\\uffff0-9]+)';
const domain = '(?:\\.(?:[a-z\\u00a1-\\uffff0-9]-*)*[a-z\\u00a1-\\uffff0-9]+)*';
const tld = `(?:\\.${options.strict ? '(?:[a-z\\u00a1-\\uffff]{2,})' : `(?:${tlds.sort((a, b) => b.length - a.length).join('|')})`})\\.?`;
const port = '(?::\\d{2,5})?';
const path = '(?:[/?#][^\\s"]*)?';
const regex = `(?:${protocol}|www\\.)${auth}(?:localhost|${ip}|${host}${domain}${tld})${port}${path}`;

Using a long string to make it pass through this regex will lead to Denial of Service.

馃悰 Proof of Concept (PoC) *

require('url-regex')({ strict: false }).test('018137.113.215.4074.138.129.172220.179.206.94180.213.144.175250.45.147.1364868726sgdm6nohQ')

馃敟 Proof of Fix (PoF) *

Screenshot from 2020-08-18 21-12-45

As the used Regex is perfect to validate URLs but just vulnerable to ReDoS, I implemented node-re2 instead of the JavaScript RegExp() function as re2 can convert a vulnerable Regex pattern to a safe one preventing any backtracking regular expressions/attacks.

馃摎 Reference:

馃憤 User Acceptance Testing (UAT)

Replaced the usage of RegExp() function with a safer regex binding node-re2.

mufeedvh and others added 2 commits August 18, 2020 21:08
Fixed ReDoS (Regex Denial of Service)
@niftylettuce
Copy link
Collaborator

Just use https://github.com/niftylettuce/url-regex-safe. It's the official resolution in the CVE/Snyk advisory.

@niftylettuce
Copy link
Collaborator

I would hope you donate that bug bounty reward to a charity @mufeedvh

@JamieSlome
Copy link

@kevva - any updates on this?

Cheers! 馃嵃

@niftylettuce
Copy link
Collaborator

Repository owner locked as resolved and limited conversation to collaborators Jan 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants