-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Multipart: Final boundary missing. #1471
Comments
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 |
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? |
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 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. |
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 |
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 |
Reopening for further investigation. Thanks for the report @tomsommer. |
Hello, Sorry if this has been said already but I could not see it here. Could someone please clarify which should be the workaround to this bug ? Regards. |
How come this prevents server from processing the request? This must not happen. |
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". |
Still having this same issue. Have set There definitely needs to be a way to disable this if there is no fix. |
I would recommend against using But this isn't because of the multipart end-boundary use case that prompted this issue. In general if you are using The only time I would ever even consider using I will make a few technical observations in a separate comment momentarily. |
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:
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. |
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:
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. |
How looks like your request which triggers this error? For eg. a curl request...? |
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
Appending another copy of the string |
But... where is the final boundary? Final boundary ends with
(if the boundary in CT header is |
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 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. |
(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:
|
Oh, sorry, now I see your point.
sorry to ask but then would you allow any other broken body payload? A wrong JSON? A non-well-formed XML? |
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. |
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. |
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: #1471 (comment) ). |
We are talking about the absence of two hyphens (* Previously I was able to bypass the parsing error by inserting another newline, boundary without |
I have the same problem as described in the opening post.
I do the upload as follows:
Okay, so that the upload works, I increase
And it works! 🎉 But what happens if I want to upload a 1 GB file? Or even a 50 GB file? I would then have to set the limit extremely high for this to work. I would therefore be interested to know: How do others handle this? Have you simply set (In my case, this was triggered by a Nextcloud client) |
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:
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
The text was updated successfully, but these errors were encountered: