Skip to content
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

SecRequestBodyAccess Off doesn't fully disable request body processing #2273

Open
slinikma opened this issue Feb 28, 2020 · 2 comments
Open
Assignees
Labels
3.x Related to ModSecurity version 3.x

Comments

@slinikma
Copy link

Hi!

Why setting SecRequestBodyAccess Off doesn't fully disable parsing request body and filling ModSecurity variables?

My case:
I'm using ModSecurity with Nginx for proxying files as multipart/form-data. I don't want to process body content and save memory,
so I set SecRequestBodyAccess to Off and when I send a big file I get bad_alloc error and core dump:

(gdb) bt full
#0 0x00007f939d2b9377 in raise () from /usr/lib64/libc.so.6
No symbol table info available.
#1 0x00007f939d2baa68 in abort () from /usr/lib64/libc.so.6
No symbol table info available.
#2 0x00007f939b74d7d5 in __gnu_cxx::__verbose_terminate_handler() ()
from /usr/lib64/libstdc++.so.6
No symbol table info available.
#3 0x00007f939b74b746 in ?? () from /usr/lib64/libstdc++.so.6
No symbol table info available.
#4 0x00007f939b74b773 in std::terminate() () from /usr/lib64/libstdc++.so.6
No symbol table info available.
#5 0x00007f939b74b993 in __cxa_throw () from /usr/lib64/libstdc++.so.6
No symbol table info available.
#6 0x00007f939b74bf2d in operator new(unsigned long) () from /usr/lib64/libstdc++.so.6
No symbol table info available.
#7 0x00007f939b7aaa19 in std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) () from /usr/lib64/libstdc++.so.6
No symbol table info available.
#8 0x00007f939b7aabd6 in std::string::_M_mutate(unsigned long, unsigned long, unsigned long) () from /usr/lib64/libstdc++.so.6
No symbol table info available.
#9 0x00007f939b7ab19e in std::string::_M_replace_safe(unsigned long, unsigned long, char const*, unsigned long) () from /usr/lib64/libstdc++.so.6
No symbol table info available.
#10 0x00007f939f364683 in modsecurity::AnchoredVariable::set(std::string const&, unsigned long) () at anchored_variable.cc:71
No locals.
#11 0x00007f939f355155 in modsecurity::Transaction::processRequestBody() ()
at transaction.cc:873
Python Exception <class 'gdb.error'> There is no member or method named _M_head_impl.: 
a = 
^[[ fullRequest = "Accept: */*\nContent-Length: 1048586473\nContent-Type: multipart/form-data; boundary=yBnB6soCHCHTWWhy6kHZ-T6lUqGnDkGXJdrd\nConnection: Keep-Alive\nUser-Agent: Apache-HttpClient/4.5".. 
l = std::vector of length 7, capacity 8 = {0x9e6df0, 0x9e6d70, 0x9e6cf0, 
0x9e6c90, 0x9e6c30, 0x9e5a30, 0x9e5b40}
#12 0x000000000055111f in ngx_http_modsecurity_pre_access_handler ()
No symbol table info available.

For me it's seems like I could have to have setting to fully disable body processing.

I've analyzed code a bit and found available memory checking condition:
https://github.com/SpiderLabs/ModSecurity/blob/v3/master/src/transaction.cc#L977

But following REQUEST_BODY variable setting doubles occupied memory size what leads bad_alloc error:
https://github.com/SpiderLabs/ModSecurity/blob/v3/master/src/anchored_variable.cc#L71

If it's a bug I could contribute and make pull request.
But now for me it's not really obvious what approach is better to fix it.

@slinikma slinikma closed this as completed Mar 2, 2020
@zimmerle
Copy link
Contributor

zimmerle commented Mar 2, 2020

Hi @slinikma,

That is something that only happens within the intermediate file? or something that happens via memory exchange?

@zimmerle zimmerle reopened this Mar 2, 2020
@zimmerle zimmerle self-assigned this Mar 2, 2020
@zimmerle zimmerle added the 3.x Related to ModSecurity version 3.x label Mar 2, 2020
@slinikma
Copy link
Author

slinikma commented Mar 3, 2020

@zimmerle, sorry I mixed up SecRequestBodyAccess Off and ctl:requestBodyAccess=Off. So, core dump I described above was with SecRequestBodyAccess set On and rules with ctl:requestBodyAccess set Off.
In such case it's not really obvious that ModSecurity set request body to REQUEST_BODY and FULL_REQUEST variables and occupies memory size three times (?) bigger than file size. Setting variables happens without available memory checking (std::string::reserve for example) and it could lead to core dump.

As for SecRequestBodyAccess On it prevents setting REQUEST_BODY and FULL_REQUEST, so if available server memory is going to end while file downloading, nginx worker process could be killed by OOM killer. But it's a problem with server resource sizing, not with ModSecurity. I just met this problem on dev server.

As for your question - nginx dumps file on disk and nginx-modsecurity connector calls msc_request_body_from_file function.

Now I don't think that my problem is a bug, but I didn't expect core dump in such case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x
Projects
None yet
Development

No branches or pull requests

2 participants