owasp-modsecurity / ModSecurity

ModSecurity is an open source, cross platform web application firewall (WAF) engine for Apache, IIS and Nginx. It has a robust event-based programming language which provides protection from a range of attacks against web applications and allows for HTTP traffic monitoring, logging and real-time analysis.
https://www.modsecurity.org
Apache License 2.0
7.7k stars 1.54k forks source link

Multipart: Final boundary missing. #1471

Open tomsommer opened 6 years ago

tomsommer commented 6 years ago

I have a customer uploading a 200mb mp4 file through AJAX in Drupal.

With mod_security2 enabled, this results in a stopped 57% upload. The following is visible in the debug log:

[26/Jun/2017:09:55:17 +0200] [XXXX][1] Request body (Content-Length) is larger than the configured limit (134217728).
[26/Jun/2017:09:55:17 +0200] [XXXX][4] Second phase starting (dcfg 6d31318).
[26/Jun/2017:09:55:17 +0200] [XXXX][1] Request body (Content-Length) is larger than the configured limit (134217728).
[26/Jun/2017:09:55:17 +0200] [XXXX][4] Input filter: Reading request body.

....

[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][9] Input filter: Bucket type HEAP contains 8000 bytes.
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][9] Input filter: Bucket type HEAP contains 192 bytes.
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][9] Input filter: Bucket type HEAP contains 6408 bytes.
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][9] Input filter: Bucket type HEAP contains 7300 bytes.
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][9] Input filter: Bucket type HEAP contains 7300 bytes.
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][4] Multipart parsing error: Multipart: Final boundary missing.
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][4] Input filter: Completed receiving request body (length 134219089).
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][4] Starting phase REQUEST_BODY.
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][9] This phase consists of 0 rule(s).
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][4] Hook insert_filter: Adding input forwarding filter (r 6d14150).
[26/Jun/2017:09:55:18 +0200] [XXXXlZepdfwwbDzafVgPaKmpJnA][4] Hook insert_filter: Adding output filter (r 6d14150).

The length in the debug is wrong, which is probably why the boundary is missing. Why does the upload stop at 134219089 bytes? The file is 243.376.128 bytes

It looks like it does partial processing, but that's not good when the partial processing results in a boundary error which kills the request? There should be a SecRequestBodyLimitAction Skip

zimmerle commented 6 years ago

Hi @tomsommer,

Please check your ModSecurity configuration file, there is this SecRequestBodyLimit that you may want to double check the value. You can check it here: https://github.com/SpiderLabs/ModSecurity/blob/v2/master/modsecurity.conf-recommended#L38

tomsommer commented 6 years ago

I already did, but the fact that that might not be enough is not good. Basically modsecurity will break any 1GB+ uploads because you cannot disable partial processing?

victorhora commented 6 years ago

Hummm @tomsommer I know there were some issues related to that on older versions of ModSecurity but I haven't heard of such issue with current builds.

Maybe check if the customer is running 2.9.1 or later build from master and also making sure that APR and other dependencies are ok (e.g. loading different version of APR from the build can lead to weird issues). You can check that on the first few lines of the error.log on startup.

I would also suggest checking and experimenting with the following directives which could impact with large file uploads:

SecRequestBodyAccess SecRequestBodyInMemoryLimit SecRequestBodyLimit SecRequestBodyLimitAction

Note there's also a hard limit of 1GiB for the maximum request body size ModSecurity will accept for buffering, excluding the size of any files being transported in the request. This can be tuned with SecRequestBodyNoFilesLimit. Even though your issue happens when you're actually uploading a file, I'm wondering if for any reason you might be hitting this limit when you're trying your file upload...

tomsommer commented 6 years ago

Running modsecurity-2.9.1, apr-1.5.2, apr-util-1.5.4, httpd-2.4.25

The problem is that the limit cuts off the request and so processing is done on what is naturally a broken request.

If you set SecRequestBodyLimit to something like 1kb and start uploading, it would in theory be reproducible?

tomsommer commented 5 years ago

Also a problem in 2.9.3

There is no way to bypass this check: https://github.com/SpiderLabs/ModSecurity/blob/b392a1ca36181a786a6d68b6eab57a8bb0bd558e/src/request_body_processor/multipart.cc#L1050

victorhora commented 5 years ago

Reopening for further investigation. Thanks for the report @tomsommer.

andrelop-zz commented 3 years ago

Hello,

Sorry if this has been said already but I could not see it here. This bug is marked with a workaround available tag but I'm failing to see which is actually the workaround to be used in this case.

Could someone please clarify which should be the workaround to this bug ?

Regards.

celesteking commented 2 years ago

How come this prevents server from processing the request? This must not happen. It's spitting HTTP 500 instead of just carrying on, even though SecRequestBodyLimitAction has been set to ProcessPartial. This is idiotic. Either remove the SecRequestBodyLimitAction completely or fix this one. You can't advertise in docs that the request will pass on if there's another limitation in software that prevents that from happening.

gmariani commented 2 years ago

Setting SecRequestBodyLimitAction to ProcessPartial did nothing, still got the error. Had to bump up SecRequestBodyLimit to get past the error. As celesteking said, either fix it or remove the "feature".

s1rc commented 2 years ago

Still having this same issue. Have set SecRequestBodyLimitAction to ProcessPartial and set SecRequestBodyLimit & SecRequestBodyNoFilesLimit to a reasonable limit and even an arbitrarily high limit with no change.

There definitely needs to be a way to disable this if there is no fix.

martinhsv commented 2 years ago

I would recommend against using SecRuleEngine On with SecRequestBodyLimitAction ProcessPartial

But this isn't because of the multipart end-boundary use case that prompted this issue.

In general if you are using SecRuleEngine On it means that you want to prevent suspected attacks from being processed by your web server. There are obvious security consequences with actively looking for attack strings in bytes 1 to n of a request body, but then ignoring those same attack strings if they are present in bytes n+1 and beyond. Even when truncation of the request body does not lead to any negative parsing outcomes (e.g. application/x-www-form-urlencoded), actively ignoring a portion of the request body creates an opportunity for evasion and exploitation.

The only time I would ever even consider using SecRequestBodyLimitAction ProcessPartial is when ModSecurity is in its early stages of a deployment and one is using SecRuleEngine DetectionOnly. In this situation, there is at least a marginal case to be made that a) the configuration is not intended to block attacks anyway and b) there may be some advantage to limiting any risk of processing delays.

I will make a few technical observations in a separate comment momentarily.

martinhsv commented 2 years ago

The basic approach to setting SecRequestBodyLimit and using ProcessPartial is to truncate and pretend that the body actually ended at byte 'n'.

Although the origins of the implementation predate my involvement in the project by a decade and a half, it looks to me like it was intended to be an advanced configuration option that could be useful to some people who know what they're doing and are prepared to deal with the processing consequences.

It may be tempting to say that ProcessPartial ought to imply that ModSecurity should cleanly cancel any codepaths that are a result of that truncation.

However, if we were to make the change suggested here, what other parsing issues should ModSecurity also be expected to deal with: 1) should we consider the last boundary that we did see to be a substitute 'final' boundary? If so does that mean that a truncated part that follows it should signal MULTIPART_DATA_AFTER? 2) If the answer to (1) is no, then what if the truncated part at the end of the body is actually truncated in the middle of the 'Content-Disposition:' header name? 3) And what if it is truncated in the middle of a quoted name= or filename= ? 4) What about a truncated json body that results in broken json parsing? 5) What about a truncated xml body that results in broken xml parsing?

By posing the above sample questions I don't mean to imply that it is impossible to decide upon answers to them, but to illustrate that:

I do grant that there is one difference with 'Final boundary missing' as compared with most of the other items illustrated in the list (1)-(5) above. Namely with most other truncation-related effects it is possible to ignore the normal effect by removing a rule -- e.g. testing REQBODY_ERROR (rule 2000002 in modsecurity.conf-recommended) However, I'm not sure it is wise to encourage a configuration that requires users to regularly disable important rules; this sort of approach can make it too easy for users to use ModSecurity incorrectly or unsafely.

There is, nevertheless, a reasonable argument to be made that some change is worth considering (perhaps even something as minor as enhancing the documentation). I will leave this open on that basis.

EvanBalster commented 1 year ago

I'm running into issues with multipart parsing after my hosting provider upgraded to a more recent version of ModSecurity. Users of my software make small (<500kb) multipart POST uploads, which due to an implementation error on my part do not include the final boundary marker. Previously, this defect was silently tolerated by ModSecurity, Apache and PHP. Recently these uploads all began to fail with 500 errors, producing the log message:

ModSecurity: Multipart parsing error: Multipart: Final boundary missing.

While I'm able to fix the implementation error on the client side, hundreds of my users will continue running older versions of the software. This affects my diagnostics system so I can't collect any information on these users' difficulties. As far as I can tell, there's no way to revert ModSecurity to the old behavior, aside from disabling inspection of the body with the technique discussed above or disabling ModSecurity entirely.

airween commented 1 year ago

How looks like your request which triggers this error? For eg. a curl request...?

EvanBalster commented 1 year ago

Here's a sample request body which will produce a 500 error on my Dreamhost VPS with Apache. The target PHP script is not run. All newlines are \r\n. The actual request has about two dozen fields and I've removed most of them for brevity.


--tattle-boundary-174094882455
Content-Disposition: form-data; name="app-arch"

x86_64
--tattle-boundary-174094882455
Content-Disposition: form-data; name="app-error-hash"

FAKERS
--tattle-boundary-174094882455
Content-Disposition: form-data; name="app-error-message"

fake error
--tattle-boundary-174094882455

Appending another copy of the string \r\n--tattle-boundary-174094882455\r\n at the end of the request satisfies ModSecurity, but I can't make this change retroactively, so all requests from old clients are being summarily blocked.

airween commented 1 year ago

But... where is the final boundary? Final boundary ends with --, so the expected value is (in this case):

--tattle-boundary-174094882455--

(if the boundary in CT header is tattle-boundary-174094882455, without leading --).

EvanBalster commented 1 year ago

The request body I posted is malformed because it lacks the final boundary. Previous versions of ModSecurity tolerated this omission. The version of ModSecurity currently used by my hosting provider will accept the request if I add an additional instance of --tattle-boundary-174094882455 (even without trailing --).

I understand the importance of validating requests, but the issue is that there appears to be no way to configure ModSecurity to tolerate this missing piece (essentially a truncation of the body). From what I gather, my only recourse is to stop ModSecurity from attempting to parse the request body at all.

I can't fix the requests being sent by old versions of my software, which will be in use for years to come. A similar issue was reported with the Xerox DocuShare client application: https://sourceforge.net/p/mod-security/mailman/message/27211899/

martinhsv commented 1 year ago

@EvanBalster ,

(Re: "Previous versions of ModSecurity tolerated this omission." You must be referring to very old versions of ModSecurity, I think.)

In general, you're right. Unless you're willing to use detection-only for the problem requests, I am not aware of a way to have such problem requests get successfully parsed as multipart requests -- short of modifying the code, of course.

Partial workarounds would depend on what your goals are, but there are some things you could try: 1) You could create a phase:1 rule that sets ctl:ruleEngine=detectionOnly. This could be particularly helpful if only some requests have this problem and you can tell via URI or request headers when it is a problem. This option will sacrifice being able to do any 'deny' results in those problem requests. 2) Use a phase:1 rule that sets ctl:requestBodyProcessor=URLENCODED. This would likely enable at least some of the detections in your rules to still work, but you would likely see more false positives that you would have to mitigate.

airween commented 1 year ago

The request body I posted is malformed because it lacks the final boundary.

Oh, sorry, now I see your point.

but the issue is that there appears to be no way to configure ModSecurity to tolerate this missing piece

sorry to ask but then would you allow any other broken body payload? A wrong JSON? A non-well-formed XML?

martinhsv commented 1 year ago

Or, building on my suggestion (1) above:

You would probably also have turn off rules that test REQBODY_ERROR, but that's doable. And this solution would probably retain most other detections.

EvanBalster commented 1 year ago

@airween

but the issue is that there appears to be no way to configure ModSecurity to tolerate this missing piece

sorry to ask but then would you allow any other broken body payload? A wrong JSON? A non-well-formed XML?

The request in question is broken only by omission of some trailing hypens that don't convey any additional information about the request. Robustness in this case does not require any malformed or extraneous information to be disregarded, and any message malformed in this way could be deterministically repaired (by completing the final boundary), so I would place this in a different category than the examples you mention.

At the moment, the lowest-impact method to stop my users' data from being lost is to configure ModSecurity to accept all defects in request bodies, including the ones you mention and many others. This opens a significantly bigger security hole than is necessary. If there were some configuration that could tolerate this missing terminator (or the general case of a truncated request) while enforcing any rules against the presence of malformed data, the attack surface would be greatly mitigated, no?

Thanks for the tips, @martinhsv; My hosting provider doesn't let me configure ModSecurity myself (short of switching it off for my entire website) so I'll need to relay information about any workarounds to their support team.

martinhsv commented 1 year ago

@EvanBalster ,

Re: "... so I would place this in a different category than the examples you mention."

The absence of a proper final boundary in a multipart request makes it clearly malformed -- and in way that is fairly comparable to a missing closing brace in a json payload or a missing closing tag in an xml document.

The main distinguishing factor (to my mind at least) is that, if you wish to accept such malformed json or xml request bodies, it's relatively easy to turn off the ModSecurity rejection, whereas that is problematic for a missing multipart final boundary (this distinguishing factor was raised in the penultimate paragraph here: https://github.com/SpiderLabs/ModSecurity/issues/1471#issuecomment-1042979122 ).

EvanBalster commented 1 year ago

@martinhsv

We are talking about the absence of two hyphens -- on the final boundary. * ModSecurity seems to afford flexibility in so many other cases, but here the only way I can stop this request blackout is to compromise most or all of its functionality.

( Previously I was able to bypass the parsing error by inserting another newline, boundary without* -- and newline at the end of the file. This might be indicative of a separate bug or vulnerability because it still wasn't a correctly-formed request.)