hgoebl / mobile-detect.js

Device detection (phone, tablet, desktop, mobile grade, os, versions)
http://hgoebl.github.io/mobile-detect.js/
MIT License
4.12k stars 892 forks source link

Vulnerable Regular Expression #67

Closed cristianstaicu closed 6 years ago

cristianstaicu commented 7 years ago

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:

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

hgoebl commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

@hgoebl can you assign me to this?

hgoebl commented 7 years ago

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 commented 7 years ago

@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 commented 6 years ago

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.

johnstew commented 6 years ago

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

hgoebl commented 6 years ago

fixed in 1.4.0