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

Email Regex Vulnerable to ReDoS attack #2609

Closed
jacob-leger opened this issue Jul 26, 2023 · 14 comments · Fixed by #2824
Closed

Email Regex Vulnerable to ReDoS attack #2609

jacob-leger opened this issue Jul 26, 2023 · 14 comments · Fixed by #2824

Comments

@jacob-leger
Copy link

Email Regex Vulnerable to ReDoS attack

The problem

When trying to develop authentication in my new application, I decided to use Zod for (part of) my validation checking.

I always check for vulnerabilities, and I know that ReDoS (Regular Expression Denial of Service) is a primarily common vulnerability. I knew Zod must use regex to validate z.string().email(). So, I checked the source code. I found (on my local machine, ./node_modules/zod/lib/types.js had a vulnerable regex.

On line 324 it has the following code:

const emailRegex = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\])|(\[IPv6:(([a-f0-9]{1,4}:){7}|::([a-f0-9]{1,4}:){0,6}|([a-f0-9]{1,4}:){1}:([a-f0-9]{1,4}:){0,5}|([a-f0-9]{1,4}:){2}:([a-f0-9]{1,4}:){0,4}|([a-f0-9]{1,4}:){3}:([a-f0-9]{1,4}:){0,3}|([a-f0-9]{1,4}:){4}:([a-f0-9]{1,4}:){0,2}|([a-f0-9]{1,4}:){5}:([a-f0-9]{1,4}:){0,1})([a-f0-9]{1,4}|(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2})))\])|([A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])*(\.[A-Za-z]{2,})+))$/;

I copied the regular expression and checked if it was vulnerable to a ReDoS attack. I went to the most recommended ReDoS checker, recheck. This is what I found:

zod-redos-proof-1

The other regexes

I have not checked any other regexes, but there are multiple others, but I will paste the code for anyone who would like to check. This starts in the same file previously mentioned but on line 320.

const cuidRegex = /^c[^\s-]{8,}$/i;
const cuid2Regex = /^[a-z][a-z0-9]*$/;
const ulidRegex = /[0-9A-HJKMNP-TV-Z]{26}/;
const uuidRegex = /^([a-f0-9]{8}-[a-f0-9]{4}-[1-5][a-f0-9]{3}-[a-f0-9]{4}-[a-f0-9]{12}|00000000-0000-0000-0000-000000000000)$/i;
const emailRegex = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\])|(\[IPv6:(([a-f0-9]{1,4}:){7}|::([a-f0-9]{1,4}:){0,6}|([a-f0-9]{1,4}:){1}:([a-f0-9]{1,4}:){0,5}|([a-f0-9]{1,4}:){2}:([a-f0-9]{1,4}:){0,4}|([a-f0-9]{1,4}:){3}:([a-f0-9]{1,4}:){0,3}|([a-f0-9]{1,4}:){4}:([a-f0-9]{1,4}:){0,2}|([a-f0-9]{1,4}:){5}:([a-f0-9]{1,4}:){0,1})([a-f0-9]{1,4}|(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2})))\])|([A-Za-z0-9]([A-Za-z0-9-]*[A-Za-z0-9])*(\.[A-Za-z]{2,})+))$/;
const emojiRegex = /^(\p{Extended_Pictographic}|\p{Emoji_Component})+$/u;
const ipv4Regex = /^(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))$/;
const ipv6Regex = /^(([a-f0-9]{1,4}:){7}|::([a-f0-9]{1,4}:){0,6}|([a-f0-9]{1,4}:){1}:([a-f0-9]{1,4}:){0,5}|([a-f0-9]{1,4}:){2}:([a-f0-9]{1,4}:){0,4}|([a-f0-9]{1,4}:){3}:([a-f0-9]{1,4}:){0,3}|([a-f0-9]{1,4}:){4}:([a-f0-9]{1,4}:){0,2}|([a-f0-9]{1,4}:){5}:([a-f0-9]{1,4}:){0,1})([a-f0-9]{1,4}|(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2}))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([0-9]{1,2})))$/;
const datetimeRegex = (args) => {
    if (args.precision) {
        if (args.offset) {
            return new RegExp(`^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{${args.precision}}(([+-]\\d{2}(:?\\d{2})?)|Z)$`);
        }
        else {
            return new RegExp(`^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\\.\\d{${args.precision}}Z$`);
        }
    }
    else if (args.precision === 0) {
        if (args.offset) {
            return new RegExp(`^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(([+-]\\d{2}(:?\\d{2})?)|Z)$`);
        }
        else {
            return new RegExp(`^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z$`);
        }
    }
    else {
        if (args.offset) {
            return new RegExp(`^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(\\.\\d+)?(([+-]\\d{2}(:?\\d{2})?)|Z)$`);
        }
        else {
            return new RegExp(`^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(\\.\\d+)?Z$`);
        }
    }
};

Please let me know of any way that I can help

@jacob-leger
Copy link
Author

I might just be stupid, but this might be an example: github.com/colinhacks/zod/issues/2580#issue-1805042689

@mikitasolo
Copy link

mikitasolo commented Aug 14, 2023

I've checked all other regexes and it seems like they are safe. Actually the latest codebase has emailRegex already changed in #2157, though the new regex is unsafe as well... There is a proposal to use HTML input regex in comments #2157 (comment) and I used it as a solution while zod doesn't have safe email regex. Just change z.email() to z.regex(/^[a-zA-Z0-9.!#$%&’*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$/) in your code as a temp solution

@jacob-leger
Copy link
Author

Thank you! 👍

@jacob-leger
Copy link
Author

Should I close, and if so, should I close as "completed" or "not planned"?

@Zertz
Copy link

Zertz commented Sep 27, 2023

We run into this problem in production and confirmed the problem by adding aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@example.com0 (yes, the 0 at the end is intentional) to the invalidEmails array in string.test.ts.

Here's a few (rudimentary) benchmarks I ran using the format above with varying lengths:

Email Seconds
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@example.com0 Unknown (tests hang)
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@example.com0 30
aaaaaaaaaaaaaaaaaaaaaaaaaaaa@example.com0 10
aaaaaaaaaaaaaaaaaaaaaaaaaa@example.com0 5

@ben-clarke
Copy link

ben-clarke commented Sep 29, 2023

This is the cause of a SNYK issue now https://security.snyk.io/vuln/SNYK-JS-ZOD-5925617 so probably means this should be fixed as it will effect many projects.

We will be looking to create a custom schema for now using ^(?!\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$ (pending further checks this works correctly) and replacing any zod email usages until there is a fix for it here.

Edit: using /^(?!\.)(?!.*\.\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$/i in place of the above after comment from @MacsDickinson

@MacsDickinson
Copy link
Contributor

adapted the regex above slightly to exclude emails containing .. and have a PR in for you

@jacob-leger
Copy link
Author

Thank you!

@matjaeck
Copy link

matjaeck commented Oct 2, 2023

This is the cause of a SNYK issue now https://security.snyk.io/vuln/SNYK-JS-ZOD-5925617 so probably means this should be fixed as it will effect many projects.

We will be looking to create a custom schema for now using ^(?!\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$ (pending further checks this works correctly) and replacing any zod email usages until there is a fix for it here.

Edit: using /^(?!\.)(?!.*\.\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$/i in place of the above after comment from @MacsDickinson

Sorry but this report and this organization as a whole is just as reliable / trustworthy as your local spam mail operator.

Even if there is a vulnerability as is described here, the report does not specify any relation to zod whatsoever.

@ben-clarke
Copy link

This is the cause of a SNYK issue now https://security.snyk.io/vuln/SNYK-JS-ZOD-5925617 so probably means this should be fixed as it will effect many projects.
We will be looking to create a custom schema for now using ^(?!\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$ (pending further checks this works correctly) and replacing any zod email usages until there is a fix for it here.
Edit: using /^(?!\.)(?!.*\.\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$/i in place of the above after comment from @MacsDickinson

Sorry but this report and this organization as a whole is just as reliable / trustworthy as your local spam mail operator.

Even if there is a vulnerability as is described here, the report does not specify any relation to zod whatsoever.

Please see the PR #2824 by @MacsDickinson - this explains (and fixes) the issue in more detail. Regardless of how reliable / trustworthy the Snyk report is, #2824 demonstrates the inefficiency in the email regex currently being used within zod.

@matjaeck
Copy link

matjaeck commented Oct 2, 2023

This is the cause of a SNYK issue now https://security.snyk.io/vuln/SNYK-JS-ZOD-5925617 so probably means this should be fixed as it will effect many projects.
We will be looking to create a custom schema for now using ^(?!\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$ (pending further checks this works correctly) and replacing any zod email usages until there is a fix for it here.
Edit: using /^(?!\.)(?!.*\.\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$/i in place of the above after comment from @MacsDickinson

Sorry but this report and this organization as a whole is just as reliable / trustworthy as your local spam mail operator.
Even if there is a vulnerability as is described here, the report does not specify any relation to zod whatsoever.

Please see the PR #2824 by @MacsDickinson - this explains (and fixes) the issue in more detail. Regardless of how reliable / trustworthy the Snyk report is, #2824 demonstrates the inefficiency in the email regex currently being used within zod.

Indeed, and thanks @MacsDickinson for working on it. My issue is that this organization repeatedly published reports about packages that contain insufficient or wrong information and often enough then become stale, spreading misinformation and making it hard to convince users an issue is fixed / does not exist.

@MacsDickinson
Copy link
Contributor

This is the cause of a SNYK issue now https://security.snyk.io/vuln/SNYK-JS-ZOD-5925617 so probably means this should be fixed as it will effect many projects.

We will be looking to create a custom schema for now using ^(?!\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$ (pending further checks this works correctly) and replacing any zod email usages until there is a fix for it here.

Edit: using /^(?!\.)(?!.*\.\.)([A-Z0-9_+-\.]*)[A-Z0-9_+-]@([A-Z0-9][A-Z0-9\-]*\.)+[A-Z]{2,}$/i in place of the above after comment from @MacsDickinson

Sorry but this report and this organization as a whole is just as reliable / trustworthy as your local spam mail operator.

Even if there is a vulnerability as is described here, the report does not specify any relation to zod whatsoever.

Please see the PR #2824 by @MacsDickinson - this explains (and fixes) the issue in more detail. Regardless of how reliable / trustworthy the Snyk report is, #2824 demonstrates the inefficiency in the email regex currently being used within zod.

Indeed, and thanks @MacsDickinson for working on it. My issue is that this organization repeatedly published reports about packages that contain insufficient or wrong information and often enough then become stale, spreading misinformation and making it hard to convince users an issue is fixed / does not exist.

In this instance the Snyk vulnerability was spot on and I was able to replicate using the info in the CVE.

@Retr02332
Copy link

Retr02332 commented Oct 4, 2023

A long time ago our security researcher found this problem and we were not notified by you.

As a security company we did not open a public ticket for a security breach, but we did send you an email. So we issued a due advisory as per our security policy:

https://fluidattacks.com/advisories/swift/

@jacob-leger
Copy link
Author

My bad...?

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 a pull request may close this issue.

7 participants