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

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

Merged
merged 10 commits into from Sep 16, 2020

Conversation

huntr-helper
Copy link
Contributor

https://huntr.dev/users/bbeale has fixed the Regular Expression Denial of Service (ReDoS) vulnerability 🔨. bbeale 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 | #6
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/urlregex/1/README.md

User Comments:

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-urlregex

⚙️ Description *

Implemented a different regex engine that is not vulnerable to the same ReDoS attacks caused by catastrophic backtracking.

💻 Technical Description *

From the readme in Google's repo:

RE2 is a fast, safe, thread-friendly alternative to backtracking regular expression engines like those used in PCRE, Perl, and Python. It is a C++ library.

Unlike the native NodeJS regex engine, which is vulnerable to denial of service attacks when a malicious user supplies a very long URL, RE2 lacks the backreference and lookahead capabilities required for this attack, making it safer to use on user supplied input. Given that this package does not accept user supplied regular expressions, and the existing regex was not making use of these operations, I imported the Node bindings for RE2, which use almost identical syntax to the native RegExp.

More information about the Node bindings can be found here.

I understand that the purpose of this package may have been to remain free of dependencies, but I believe that using a regex engine without these capabilities is the right solution, especially given that the dangerous features were not being used.

🐛 Proof of Concept (PoC) *

const urlRegex = require("urlregex");
const isValid = urlRegex().test(
  "http://huntr.devtestvulnerability2312321.testvulnerability2312321.testvulnerability2312321.testvulnerability2312321.testvulnerability2312321.testvulnerability2312321.testvulnerability2312321"
);

console.log(isValid);

🔥 Proof of Fix (PoF) / User Acceptance Testing (UAT)

Unit tests
urlregex-tests-match
urlregex-tests-notmatch

@nescalante nescalante merged commit e5a085a into nescalante:master Sep 16, 2020
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

4 participants