Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

Fixed ReDoS (Regex Denial of Service) #1

Merged
merged 1 commit into from
Aug 20, 2020
Merged

Fixed ReDoS (Regex Denial of Service) #1

merged 1 commit into from
Aug 20, 2020

Conversation

mufeedvh
Copy link

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-node-dns-sync

⚙️ Description *

The project node-dns-sync was validating hostnames with a regex vulnerable to ReDoS (Regex Denial of Service).

💻 Technical Description *

The implemented Regex pattern to validate hostnames is vulnerable to ReDoS:

/^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]).)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]*[A-Za-z0-9])$/

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

🐛 Proof of Concept (PoC) *

Refer: skoranga#5

🔥 Proof of Fix (PoF) *

As the used Regex is perfect to validate a hostname 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.

Copy link

@Mik317 Mik317 left a comment

Choose a reason for hiding this comment

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

Good fix !!!

Cheers,
Mik

@JamieSlome JamieSlome removed the request for review from toufik-airane August 20, 2020 11:08
@JamieSlome JamieSlome merged commit bad9d65 into 418sec:master Aug 20, 2020
@huntr-helper
Copy link

Congratulations mufeedvh - your fix has been selected! 🎉

Thanks for being part of the community & helping secure the world's open source code.
If you have any questions, please respond in the comments section. Your bounty is on its way - keep hunting!

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.

4 participants