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

[2609] fix ReDoS vulnerability in email regex #2824

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

MacsDickinson
Copy link
Contributor

@MacsDickinson MacsDickinson commented Sep 29, 2023

Fixes #2609

The current regex used for email validation contains "catastrophic backtracking", specifically ([A-Z0-9_+-]+\.?)*. This gets evaluated inefficiently by JS, resulting in an exponential increase in execution time for failed matches.

This can be replicated easily - here's execution time against ^([A-Z0-9_+-]+\.?)*[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$

string match execution
aaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.615s
aaaaaaaaaaaaaaaaaaaaaaaa@test.c false 1.165s
aaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 2.280s
aaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 4.507s
aaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 8.964s
aaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 18.018s
aaaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 💀

This PR provides an alternative regex. Rather that matching on the (aaaaa.)+ we instead do a negative lookahead for the presence of .. or the email starting with a .. This approach isn't susceptible to ReDoS

here's execution time against ^(?!\.)(?!.*\.\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$

regex email match execution
aaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.044s
aaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.046s
aaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.044s
aaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.046s
aaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.045s
aaaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.045s
aaaaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.044s

@netlify
Copy link

netlify bot commented Sep 29, 2023

Deploy Preview for guileless-rolypoly-866f8a ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b1f3cc0
🔍 Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/6516e56b5e364f00087df9e1
😎 Deploy Preview https://deploy-preview-2824--guileless-rolypoly-866f8a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jacob-leger
Copy link

I checked it on a popular ReDoS checker... it's completely safe!

Screenshot 2023-09-29 12 45 24 PM

@samchungy
Copy link
Contributor

samchungy commented Oct 1, 2023

Hey @colinhacks, could we please get a tiny patch release for this one 🙇

https://security.snyk.io/vuln/SNYK-JS-ZOD-5925617

@maietta
Copy link

maietta commented Oct 3, 2023

Bump!

@colinhacks colinhacks merged commit 2ba00fe into colinhacks:master Oct 3, 2023
3 checks passed
@colinhacks
Copy link
Owner

colinhacks commented Oct 3, 2023

Thanks @MacsDickinson this is massively appreciated
Landed in Zod 3.22.3

@kurtextrem
Copy link

kurtextrem commented Oct 4, 2023

May I ask what speaks against using the HTML5 spec regex instead of a hand rolled one?

An old variant is even already in the source (commented out), and PR #2157 even says:

Nice! I'd missed this. I think it makes sense and seems to be functionally equivalent to mine but with support for "printable characters" in the local part. I'm not convinced emails with those characters should be considered valid, but I like the parity with browsers.

The HTML5 spec regex is not vulnerable: https://stackoverflow.com/a/8829363.

From the previous PR, I also want to mention someone mentioned emails like O'Railey@domain.com return false currently.

@rktyt
Copy link

rktyt commented Oct 5, 2023

Could you please backport it to 3.21.4?
Because currently our code does not work in 3.22.

@fabian-hiller
Copy link
Contributor

The email regex we plan to use for Valibot is more accurate and twice as fast. If interested, I can provide more details.

screenshot

  1. Open this link: https://esbench.com/bench/6520e66d7ff73700a4deba40
  2. Navigate to Results
  3. Click RUN BENCHMARK button

@fabian-hiller
Copy link
Contributor

Here ist the final regex, we plan to use for Valibot.

@tjenkinson
Copy link

I opened #2849 to add a redos check to eslint

@tgds
Copy link

tgds commented Dec 18, 2023

Fixes #2609

The current regex used for email validation contains "catastrophic backtracking", specifically ([A-Z0-9_+-]+\.?)*. This gets evaluated inefficiently by JS, resulting in an exponential increase in execution time for failed matches.

This can be replicated easily - here's execution time against ^([A-Z0-9_+-]+\.?)*[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$

string match execution
aaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.615s
aaaaaaaaaaaaaaaaaaaaaaaa@test.c false 1.165s
aaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 2.280s
aaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 4.507s
aaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 8.964s
aaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 18.018s
aaaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 💀
This PR provides an alternative regex. Rather that matching on the (aaaaa.)+ we instead do a negative lookahead for the presence of .. or the email starting with a .. This approach isn't susceptible to ReDoS

here's execution time against ^(?!\.)(?!.*\.\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$

regex email match execution
aaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.044s
aaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.046s
aaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.044s
aaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.046s
aaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.045s
aaaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.045s
aaaaaaaaaaaaaaaaaaaaaaaaaaaaa@test.c false 0.044s

may I ask, what environment did you use to test it? I'm trying to replicate the issue, but the vulnerable expression seems to perform well anywhere I test.

@jacob-leger
Copy link

It looks safe to me!

I usually use devina when checking for redos vulnerabilities. Anyway, here's what it says:
Screenshot of devina.io redos checker in use

Btw, I've been looking for the perfect email regex for over 3 years, I'm honestly surprised that this works.

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.

Email Regex Vulnerable to ReDoS attack