Specially constructed multi-part requests cause multi-second response times; vulnerable to DoS
State Resolved (Closed)
Disclosed publicly 2018-12-05T21:46:17.275Z
Reported To
Weakness none
Bounty $1,500
Collapse


Timeline
submitted a report to Ruby on Rails .
2018-10-31T00:31:56.429Z

The multi-part body parsing in Rack and consequently Rails has a worse-than-linear performance relative to the number of parts in the request body.

In small scale (i.e. non-disruptive) tests on a variety of Rails applications on the internet, including my own, GitHub.com, Heroku API, Instacart, Shopify, Bugcrowd, and others, it was trivial to cause request servicing to take long enough to cause gateway timeouts. It would not be particularly difficult to generate enough of these requests to tie up the majority of web serving resources for a typical Rails application.

This vulnerability has also been separately disclosed to rack-core mailing list by me.

Steps To Reproduce:

I've created a script that can be run here against any Rack-based application: https://gist.github.com/bjeanes/63580e27c197885d4b07160fae132108

By default it generates a request body with 10,000 parts which, in my testing, was enough to cause GitHub API to take between 15-25 seconds to service the request once the request transfer had completed.

Addendum

Some benchmarking of Rack is included here, which was also sent to rack-core:

N = number of parts, 
boundary used is as generated by Chrome, but larger boundaries cause
higher impact

Rehearsal --------------------------------------------
N: 1       0.032670   0.006635   0.039305 (  0.044159)
N: 100     0.008245   0.001319   0.009564 (  0.009971)
N: 1000    0.149332   0.079087   0.228419 (  0.255769)
N: 2000    0.398711   0.276931   0.675642 (  0.691356)
N: 5000    2.254126   1.569181   3.823307 (  3.871649)
N: 10000   7.134949   4.350681  11.485630 ( 12.083888)
N: 20000  15.946187  10.491861  26.438048 ( 28.684177)
---------------------------------- total: 42.699915sec

               user     system      total        real
N: 1       0.000372   0.000003   0.000375 (  0.000371)
N: 100     0.004889   0.000021   0.004910 (  0.004977)
N: 1000    0.117571   0.015548   0.133119 (  0.192915)
N: 2000    0.468934   0.309703   0.778637 (  0.870675)
N: 5000    2.086055   1.482317   3.568372 (  3.830543)
N: 10000   7.110589   4.488229  11.598818 ( 12.110710)
N: 20000  14.559701   9.948678  24.508379 ( 25.537332)

This is not technically in the Rails codebase, but it is in a non-optional dependency of every Rails application. Furthermore, I did receive some feedback from core members that a Rack vulnerability is likely to be eligible for this bounty: https://twitter.com/_matthewd/status/1057266505056800768.

Discovered by Bo Jeanes (@bjeanes) and Jack Chen (@chendo) and vetted after the fact by Charlie Somerville (@charliesome).

Impact

Resource starvation of web request servicing, by causing multiple long-running requests. Attack can be constructed with just a HTML web form, making it literally click-button easy. That it can be generated from a form also has potential implications when combined with XSS or some other mechanism where an attacker could cause arbitrary user agents en masse to send such requests.

Regards,
Frans

  • 0 attachments:
matthewd Activities::Comment
2018-10-31T15:54:22.308Z
Hey Bo! Thanks for jumping through the extra reporting hoops! If it's not reproducible on master, I think this is https://github.com/rack/rack/pull/1288 -- which has its own issues, now being addressed by https://github.com/rack/rack/pull/1309. Does that sound plausible to you?


tenderlove Activities::Comment
2018-10-31T18:28:04.724Z
I mentioned this on the mailing list, but I was able to bisect the problem to this commit: https://github.com/rack/rack/pull/1192/commits/ee0174822f9c4e939bd810c3e906554a7137973d The changes on master seem too big to backport, so I'd prefer if we just revert this commit on the 2-0-stable branch. Thoughts?


bjeanes Activities::Comment
2018-10-31T20:18:19.782Z
Hi @matthewd and @tenderlove that all makes total sense. @tenderlove reverting ee0174822 will mean that https://github.com/rack/rack/issues/1075 is still a problem though. I am getting dropped as a recipient to replies on rack-core FYI. I still haven't received anything since the last message I sent. If any of it was meant for me, I'd love if you could re-send to me there. I think Google Groups is a bit weird in the presence of CCs or something...


tenderlove Activities::Comment
2018-10-31T20:59:34.189Z
> I am getting dropped as a recipient to replies on rack-core FYI. I still haven't received anything since the last message I sent. Blah, I'm sorry. I'm hitting "reply-all", so it *should* have gone to you. Maybe Mail.app is to blame. I responded with basically the same comment I put here (just reverting that one commit). > @tenderlove reverting ee0174822 will mean that https://github.com/rack/rack/issues/1075 is still a problem though. I am OK with slowing down 200MB uploads if it means avoiding DoS for small uploads. Presumably `master` will fix both cases, and people can still adjust the buffer size as demonstrated in https://github.com/rack/rack/issues/1075 until we ship master. @matthewd wdyt?


bjeanes Activities::Comment
2018-11-01T00:16:57.529Z
> Blah, I'm sorry. I'm hitting "reply-all", so it should have gone to you. No worries! I assume rack-core is set up to send messages from external people _as_ rack-core, so Mail doesn't even know that I should be a recipient. > I am OK with slowing down 200MB uploads if it means avoiding DoS for small uploads Agreed; that's reasonable. IME, you're better off uploading large files somewhere else directly anyway. Incidentally, sending files to Rails instead of S3 is how I discovered this issue. We had a form on our site that POSTed an attachment but also had a bunch of generated checkboxes. A pathological case resulted in 5000+ checkboxes + a file field, and submitting that form was causing 504s. What do you think about yanking 2.0.4 & 2.0.5? I was able to reproduce this on a scary number of high-profile targets (GitHub API, Heroku API, Shopify, etc). Esp given the fact that #1288 was opened publicly, a malicious hacker is probably pretty likely to discover this.


bjeanes Activities::Comment
2018-11-01T00:27:21.538Z
> and people can still adjust the buffer size as demonstrated in https://github.com/rack/rack/issues/1075 until we ship master. If I understand the issue correctly, they can't do so without making themselves vulnerable to this type of DoS, even for versions older than 2.0.4, right? I can't do so right now, but I'll be happy to run my benchmark tonight on older Racks with the manual buffer size bump to confirm this. If you choose to post a security advisory about these findings, it may be worth warning against increasing the buffer size on older versions, if this is indeed the case.


tenderlove Activities::Comment
2018-11-01T15:22:33.790Z
> If I understand the issue correctly, they can't do so without making themselves vulnerable to this type of DoS, even for versions older than 2.0.4, right? I can't do so right now, but I'll be happy to run my benchmark tonight on older Racks with the manual buffer size bump to confirm this. You don't need to. I confirmed that changing the buffer size is the only thing that introduces this. I don't think people should do it, I'm just saying if a user cares about 200MB uploads more than avoiding a DoS, then by all means. I just wouldn't recommend it. > What do you think about yanking 2.0.4 & 2.0.5? I was able to reproduce this on a scary number of high-profile targets (GitHub API, Heroku API, Shopify, etc). Esp given the fact that #1288 was opened publicly, a malicious hacker is probably pretty likely to discover this. We don't typically yank gems in the case of a security release. It will break builds, and yanking the gem won't cause people who are running it in production to stop running it. Lets just get a fix / CVE shipped ASAP. I'll write a CVE, and we should be able to ship this either Friday or Monday. I dislike shipping on a Friday, but as you said, this is kind of public.


tenderlove Activities::Comment
2018-11-01T21:59:56.708Z
Here's the CVE so far: ``` There is a possible DoS vulnerability in the multipart parser in Rack. This vulnerability has been assigned the CVE identifier CVE-YYYY-XXXX. Versions Affected: 2.0.4, 2.0.5 Not affected: <= 2.0.3 Fixed Versions: 2.0.6 Impact ------ There is a possible DoS vulnerability in the multipart parser in Rack. Specially crafted requests can cause the multipart parser to enter a pathological state, causing the parser to use CPU resources disproportionate to the request size. Impacted code can look something like this: ``` Rack::Request.new(env).params ``` But any code that uses the multi-part parser may be vulnerable. All users running an affected release should either upgrade or use one of the workarounds immediately. Releases -------- The 2.0.6 release is available at the normal locations. Workarounds ----------- To work around this issue, the following code can be used: ``` require "rack/multipart/parser" Rack::Multipart::Parser.send :remove_const, :BUFSIZE Rack::Multipart::Parser.const_set :BUFSIZE, 16384 ``` Patches ------- To aid users who aren't able to upgrade immediately we have provided patches for the two supported release series. They are in git-am format and consist of a single changeset. * 2-0-multipart-dos.patch - Patch for 2.0 series Please note that only the 1.6.x and 2.0.x series are supported at present. Users of earlier unsupported releases are advised to upgrade as soon as possible as we cannot guarantee the continued availability of security fixes for unsupported releases. Credits ------- ``` I've also attached the patch. @bjeanes what should I put in Credits?


tenderlove Activities::BugTriaged
2018-11-01T22:01:30.551Z


bjeanes Activities::Comment
2018-11-01T23:33:51.072Z
@tenderlove > We don't typically yank gems in the case of a security release. It will break builds, and yanking the gem won't cause people who are running it in production to stop running it. Ack. Makes 100% sense. > Here's the CVE so far: That looks good to me, except "provided patches for the two supported release series." 1.6.x isn't vulnerable so there is only one patch. This could be a made a bit clearer, but is otherwise good IMO. > @bjeanes what should I put in Credits? * Bo Jeanes * Jack "chendo" Chen I'm not sure if you want anything other than names, but our emails are [email protected] and [email protected] if that metadata should be associated with it at all. I discovered it in my app, chendo helped isolate the cause and wrote the benchmark script. We ran our findings past Charlie Somerville (@charliesome) to help verify our intuitions that this was serious enough to merit a report since he's had more experience here. I certainly don't mind if he's credited too but I don't know if that would typically be the case.


tenderlove Activities::Comment
2018-11-01T23:37:15.963Z
> That looks good to me, except "provided patches for the two supported release series." 1.6.x isn't vulnerable so there is only one patch. This could be a made a bit clearer, but is otherwise good IMO. Good catch, I'll fix it. The whole report is just free text basically, so I can put anything. I'll put: ``` Bo Jeanes <[email protected]> Jack "chendo" Chen <[email protected]> ```


bjeanes Activities::Comment
2018-11-01T23:40:53.631Z
Sounds great. I think the CVE could also mention that older versions that have manually increased `BUFSIZE` are also at risk. If not in CVE, there may be a more appropriate channel. What do you think? I'm not sure if you plan to release master as 2.0.6 or just re-release 2.0.5 with the patch applied as 2.0.6 (you had mentioned the fix being a bit tricky to back-port so I suspect the latter), but that might be important context for the above as only the eventual version released with all of master's changes will fix both issues, right?


tenderlove Activities::Comment
2018-11-01T23:49:12.507Z
> I think the CVE could also mention that older versions that have manually increased BUFSIZE are also at risk. If not in CVE, there may be a more appropriate channel. What do you think? That sounds good, I'll add it. > I'm not sure if you plan to release master as 2.0.6 or just re-release 2.0.5 with the patch applied as 2.0.6 (you had mentioned the fix being a bit tricky to back-port so I suspect the latter), I'm going to release from the 2-0-stable branch which doesn't have the other changes. > might be important context for the above as only the eventual version released with all of master's changes will fix both issues, right? I don't think we need to mention it in the security issue. Only people dealing with very large file uploads will be impacted. Applying this security fix reintroduces that bug, but it's just a bug, and folks impacted by that bug can wait until master is released (they've been surviving just fine until version 2.0.4 anyway).


bjeanes Activities::Comment
2018-11-01T23:54:33.298Z
👍 Let me know if you need anything else from me.


tenderlove Activities::Comment
2018-11-02T23:03:23.486Z
👋 It's getting late here, so I'm going to ship this Monday morning PST. Thanks!


Activities::BountyAwarded
2018-11-05T21:42:43.459Z


tenderlove Activities::BugResolved
2018-11-05T21:42:56.099Z


tenderlove Activities::AgreedOnGoingPublic
2018-11-05T21:46:09.418Z


Activities::ReportBecamePublic
2018-12-05T21:46:17.298Z