Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Vulnerable Regular Expression #67

Closed
cristianstaicu opened this issue Sep 5, 2017 · 9 comments
Closed

Vulnerable Regular Expression #67

cristianstaicu opened this issue Sep 5, 2017 · 9 comments

Comments

@cristianstaicu
Copy link

The following regular expression used for parsing the user agent is vulnerable to ReDoS:

/Dell.*Streak|Dell.*Aero|Dell.*Venue|DELL.*Venue Pro|Dell Flash|Dell Smoke|Dell Mini 3iX|XCD28|XCD35|\b001DL\b|\b101DL\b|\bGS01\b/i

The slowdown is moderate: for 50.000 characters around 10 seconds matching time. I would suggest one of the following:

  • remove the regex,
  • anchor the regex,
  • limit the number of characters that can be matched by the repetition,
  • limit the input size.

If needed, I can provide an actual example showing the slowdown.

@hgoebl
Copy link
Owner

hgoebl commented Sep 6, 2017

Thanks for your contribution. As this project is just a "satellite" of Mobile-Detect (php) and all patterns are generated based on Mobile-Detect, we have limited power to change the RegExs.
Would you mind opening an issue in Mobile-Detect?

BTW the string used to match patterns is the user-agent string. Do you think there's still an issue?

@darrenscerri
Copy link

BTW the string used to match patterns is the user-agent string. Do you think there's still an issue?

An attacker can send a user-agent string of arbitrary length

@serbanghita
Copy link
Collaborator

@cristianstaicu @darrenscerri @hgoebl Ok, I'm limiting the .* regex. I'm taking a look at the DB and come back with an updated regex

@serbanghita
Copy link
Collaborator

@hgoebl can you assign me to this?

@hgoebl
Copy link
Owner

hgoebl commented Nov 27, 2017

IMHO we should better limit the input size.
In case of mobile-detect.js it's not desirable to have significantly longer regexps.

@serbanghita (dumb question) how can I assign this to you?

@darrenscerri
Copy link

@hgoebl I think a global limit on the input size would be more appropriate than fiddling with the regexps. I think a limit of 500 characters is quite reasonable.

serbanghita added a commit to serbanghita/Mobile-Detect that referenced this issue Dec 3, 2017
@serbanghita
Copy link
Collaborator

Guys I took a look at the User-Agent database regarding Dell and simplified the regex, I also limited the length of the User-Agent to max 500 characters.

@hgoebl hgoebl closed this as completed in 7222f6e Dec 9, 2017
@johnstew
Copy link

johnstew commented Feb 12, 2018

Is this still a vulnerability or is everything good in 1.4.1?

@hgoebl
Copy link
Owner

hgoebl commented Feb 13, 2018

fixed in 1.4.0

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants