browserslist/browserslist-useragent-regexp

Unsafe result regexp

budarin opened this issue · 8 comments

budarin commented

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

dangreen commented

@budarin Hi. Please provide example of expression.

budarin commented 
/((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 commented

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

budarin commented

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 commented

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

dangreen commented

@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 commented

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?