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
Adding ua_parser_js ReDoS Module #9284
Adding ua_parser_js ReDoS Module #9284
Conversation
"ua-parser-js" is an npm module for parsing browser user-agent strings. Vulnerable version of this module have a problematic regular expression that can be exploited to cause the entire application processing thread to "pause" as it tries to apply the regular expression to the input. This is problematic for single-threaded application environments such as nodejs. The end result is a denial of service condition for vulnerable applications, where no further requests can be processed.
He's back! |
do you mind putting some docs together for this module? Template is here |
Was this reported to the project? I dont see any obvious issues or PRs over there. |
super( | ||
'Name' => 'ua-parser-js npm module - Regular Expression Denial of Service', | ||
'Description' => %q{ | ||
This module exploits a Regular Expression Denial of Service vulnerability in the npm module "ua-parser-js". Server-side applications that use "ua-parser-js" for parsing the browser user-agent string will be vulnerable if they call the "getOS" or "getResult" functions. This vulnerability was fixed as of version 0.7.16. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you multi-line this please
|
||
def initialize | ||
super( | ||
'Name' => 'ua-parser-js npm module - Regular Expression Denial of Service', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we shorten this, maybe DoS or REDoS? Maybe also drop the - before the REDoS part as well?
}, | ||
'References' => | ||
[ | ||
['CWE', '400'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per comment in PR, was the project notified of the issue? That would give something to point back to.
|
||
def test_service_unresponsive | ||
begin | ||
print_status("Testing for service unresponsiveness.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trade " for '
}) | ||
|
||
if res.nil? | ||
print_good("Service not responding.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for interpolation
print_error("Service responded with a valid HTTP Response; ReDoS attack failed.") | ||
end | ||
rescue ::Rex::ConnectionRefused | ||
print_error("An unknown error occurred.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for interpolation
rescue ::Rex::ConnectionRefused | ||
print_error("An unknown error occurred.") | ||
rescue ::Timeout::Error | ||
print_good("HTTP request timed out, most likely the ReDoS attack was successful.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for interpolation
|
||
def test_service | ||
begin | ||
print_status("Testing Service to make sure it is working.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for interpolation
}) | ||
|
||
if !res.nil? && (res.code == 200 || res.code == 404) | ||
print_status("Test request successful, attempting to send payload") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for interpolation
end | ||
|
||
def run | ||
if !test_service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we prefer unless
instead of if !
A few things were changed as per the PR comments: 1) The module title was reworded 2) The module description was multi-lined 3) Negative logic was rewritten to use 'unless' 4) Strings which did not require interpolation were rewritten 5) Documentation markdown was added.
This issue was indeed reported to the project, but it was done privately. I added a link to the commit in the references section. |
Against the test app on ubuntu 16.04 with the vuln code: Good
success. not vuln
success. python simplehttpserver
success. |
a few quick changes, and this will be good to go tomorrow! |
What changes specifically need to be made at this point? I'm a bit confused. Sorry. |
I left 2 comments, one about the spaces before => and the other about slightly rearranging the docs |
I'm sorry, I don't see the two comments you are referring to anywhere in the pull request. What am I doing wrong? Is there something I need to do in order to be able to see your comments? |
Strange, I had the same thing happen where one tab showed the comments, and another one didn't. Regardless, I linked to the comments. Shouldn't take but 5min, and I'll land in a few hours! |
I'm really sorry to sound like a broken record, but I still cannot see the comments you have left. The links you provided don't direct me anywhere in the source tab. I've tried two computers locally and had a colleague on the other side of the country try as well, and we have not been able to see your comments. I'm really not sure what is going on at this point. I apologize for the inconvenience. |
UGHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH. |
or `getResult` functions will be vulnerable to this module. An example server is provided | ||
below. | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets change this to be more of a 'how to install' to include that example. See https://github.com/rapid7/metasploit-framework/blob/master/documentation/modules/auxiliary/scanner/gopher/gopher_gophermap.md#ubuntu-1604-install as an example. All the info is captured here though, just a little re-arranging
7. Open up a new terminal. | ||
8. Start msfconsole. | ||
9. `use auxiliary/dos/http/ua_parser_js_redos`. | ||
10. `set RHOSTS <IP>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rhost not rhosts
|
||
def initialize | ||
super( | ||
'Name' => 'ua-parser-js npm module ReDoS', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getting kinda nit picky here but can we tighten up the spaces before the =>
? i think 2 or 3 could be removed and everything still spaced out nicely
There were several formatting and layout issues that are fixed in this commit. Also changing `RHOSTS` to `RHOST`.
had 2 minor edits: 544e4e3 but just ninja patched it to prevent that from being a hangup. Thanks for the module, everything looked pretty good from the beginning, so easy land! |
Release NotesThis PR adds a DoS module against ua_parser_js which is an npm module which has a problematic regex. |
Thanks for landing this. As always, it was a pleasure! |
"ua-parser-js" is an npm module for parsing browser
user-agent strings. Vulnerable version of this module
have a problematic regular expression that can be exploited
to cause the entire application processing thread to "pause"
as it tries to apply the regular expression to the input.
This is problematic for single-threaded application environments
such as nodejs. The end result is a denial of service
condition for vulnerable applications, where no further
requests can be processed.
Verification
List the steps needed to make sure this thing works
server.js
npm i express
to install express in the test application directorynpm i ua-parser-js@0.7.15
to install a vulnerable version of ua-parser-jsnpm i ua-parser-js
to install the latest version of ua-parser-js.node server.js
msfconsole
use auxiliary/dos/http/ua_parser_js_redos
run
Test application: