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

Unsafe result regexp #753

Open
budarin opened this issue Jun 14, 2021 · 8 comments
Open

Unsafe result regexp #753

budarin opened this issue Jun 14, 2021 · 8 comments

Comments

@budarin
Copy link

budarin commented Jun 14, 2021

This tool https://github.com/substack/safe-regex says that the expression is unsafe :(

@dangreen
Copy link
Member

@budarin Hi. Please provide example of expression.

@budarin
Copy link
Author

budarin commented Jun 15, 2021

/((CPU[ +]OS|iPhone[ +]OS|CPU[ +]iPhone|CPU IPhone OS)[ +]+(14[_.]0|14[_.]([1-9]|\d{2,})|14[_.]4|14[_.]([5-9]|\d{2,})|(1[5-9]|[2-9]\d|\d{3,})[_.]\d+)(?:[_.]\d+)?)|(Opera\/.+Opera Mobi.+Version\/(12\.1|12\.([2-9]|\d{2,})|(1[3-9]|[2-9]\d|\d{3,})\.\d+|62\.0|62\.([1-9]|\d{2,})|(6[3-9]|[7-9]\d|\d{3,})\.\d+))|(Opera\/(12\.1|12\.([2-9]|\d{2,})|(1[3-9]|[2-9]\d|\d{3,})\.\d+|62\.0|62\.([1-9]|\d{2,})|(6[3-9]|[7-9]\d|\d{3,})\.\d+).+Opera Mobi)|(Opera Mobi.+Opera(?:\/|\s+)(12\.1|12\.([2-9]|\d{2,})|(1[3-9]|[2-9]\d|\d{3,})\.\d+|62\.0|62\.([1-9]|\d{2,})|(6[3-9]|[7-9]\d|\d{3,})\.\d+))|(Edge\/(90(?:\.0)?|90(?:\.([1-9]|\d{2,}))?|(9[1-9]|\d{3,})(?:\.\d+)?))|((Chromium|Chrome)\/(90\.0|90\.([1-9]|\d{2,})|(9[1-9]|\d{3,})\.\d+)(?:\.\d+)?)|(Version\/(14\.0|14\.([1-9]|\d{2,})|(1[5-9]|[2-9]\d|\d{3,})\.\d+)(?:\.\d+)? Safari\/)|(Firefox\/(88\.0|88\.([1-9]|\d{2,})|(89|9\d|\d{3,})\.\d+)\.\d+)|(Firefox\/(88\.0|88\.([1-9]|\d{2,})|(89|9\d|\d{3,})\.\d+)(pre|[ab]\d+[a-z]*)?)/

@sballesteros
Copy link

I was concerned by this issue as well but according to https://regex.rip/ the regexp is not vulnerable to exponential backtracking.

@budarin
Copy link
Author

budarin commented Dec 20, 2021

eslint warns me

Unsafe Regular Expressioneslint(security/detect-unsafe-regex)

export const supportedBrowsers =
    /((CPU[ +]OS|iPhone[ +]OS|CPU[ +]iPhone|CPU IPhone OS)[ +]+(15[_.]0|15[_.]([1-9]|\d{2,})|(1[6-9]|[2-9]\d|\d{3,})[_.]\d+)(?:[_.]\d+)?)|(Opera\/.+Opera Mobi.+Version\/(64\.0|64\.([1-9]|\d{2,})|(6[5-9]|[7-9]\d|\d{3,})\.\d+))|(Opera\/(64\.0|64\.([1-9]|\d{2,})|(6[5-9]|[7-9]\d|\d{3,})\.\d+).+Opera Mobi)|(Opera Mobi.+Opera(?:\/|\s+)(64\.0|64\.([1-9]|\d{2,})|(6[5-9]|[7-9]\d|\d{3,})\.\d+))|((?:Chrome).*OPR\/(81\.0|81\.([1-9]|\d{2,})|(8[2-9]|9\d|\d{3,})\.\d+)\.\d+)|(SamsungBrowser\/(14\.0|14\.([1-9]|\d{2,})|(1[5-9]|[2-9]\d|\d{3,})\.\d+))|(Edge\/(95(?:\.0)?|95(?:\.([1-9]|\d{2,}))?|(9[6-9]|\d{3,})(?:\.\d+)?))|((Chromium|Chrome)\/(93\.0|93\.([1-9]|\d{2,})|(9[4-9]|\d{3,})\.\d+)(?:\.\d+)?)|(Version\/(15\.1|15\.([2-9]|\d{2,})|(1[6-9]|[2-9]\d|\d{3,})\.\d+)(?:\.\d+)? Safari\/)|(Firefox\/(94\.0|94\.([1-9]|\d{2,})|(9[5-9]|\d{3,})\.\d+)\.\d+)|(Firefox\/(94\.0|94\.([1-9]|\d{2,})|(9[5-9]|\d{3,})\.\d+)(pre|[ab]\d+[a-z]*)?)/;

errors & warnings

'\d{2,}' can be replaced with '\d{2}' because of '.+'. This cannot be fixed automatically because it might change or remove a capturing group.[2, 253]

The quantifier '\d{2,}' can exchange characters with '.+'. Using any string accepted by /\d+/, this can be exploited to cause at least polynomial backtracking. This might cause exponential backtracking.[2, 253]

The quantifier '\d+' can exchange characters with '.+'. Using any string accepted by /\d+/, this can be exploited to cause at least polynomial backtracking. This might cause exponential backtracking. [2, 286]

Unexpected quantifier Non-capturing group. [2, 390]

Unexpected useless alternative. This alternative is already covered by '95(?:\.0)?' and can be removed. Careful! This alternative contains capturing groups which might be difficult to remove. [2, 558]

Unsafe Regular Expression

image

@dangreen
Copy link
Member

dangreen commented Nov 6, 2022

@budarin Hi. Please try v4.0.0-beta.1.

@dangreen dangreen closed this as completed Nov 6, 2022
@budarin
Copy link
Author

budarin commented Nov 7, 2022

@dangreen dangreen reopened this Nov 7, 2022
@dangreen
Copy link
Member

dangreen commented Nov 7, 2022

@budarin Currently, I don't know how quickly to fix this. Feel free to make a PR to https://github.com/TrigenSoftware/ua-regexes-lite

@sjparkinson
Copy link

I wonder if using https://www.npmjs.com/package/re2 would be practical to mitigate this sort of issue, or generating re2 compatible regexes in some other way?

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

No branches or pull requests

4 participants