SecRequestBodyInMemoryLimit not used in V3

[AndrewFromMelbourne]

Looking at the code for Modsecurity V3, the configuration directive
SecRequestBodyInMemoryLimit is not used. The parser accepts the value of SecRequestBodyInMemoryLimit and uses it to set the value of driver.m_requestBodyInMemoryLimit. However, driver.m_requestBodyInMemoryLimit is not used in any other part of the code.

[zimmerle]

Hi @AndrewFromMelbourne,

The request size is taken into consideration into this block here:
https://github.com/SpiderLabs/ModSecurity/blob/v3/master/src/transaction.cc#L871-L903

As you can see the values are being handled.

[AndrewFromMelbourne]

Hi @zimmerle. I think we are talking about two different configuration directives.

In seclang-scanner.ll

CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT     (?i:SecRequestBodyInMemoryLimit)

In seclang-parser.yy

    | CONFIG_DIR_REQ_BODY_IN_MEMORY_LIMIT
      {
        driver.m_requestBodyInMemoryLimit.m_set = true;
        driver.m_requestBodyInMemoryLimit.m_value = atoi($1.c_str());
      }

Searching all the files from the release for m_requestBodyInMemoryLimit

$ find . -type f | xargs grep m_requestBodyInMemoryLimit
./headers/modsecurity/rules_properties.h:    ConfigDouble m_requestBodyInMemoryLimit;
./src/parser/seclang-parser.cc:        driver.m_requestBodyInMemoryLimit.m_set = true;
./src/parser/seclang-parser.cc:        driver.m_requestBodyInMemoryLimit.m_value = atoi(yystack_[0].value.as< std::string > ().c_str());
./src/parser/seclang-parser.yy:        driver.m_requestBodyInMemoryLimit.m_set = true;
./src/parser/seclang-parser.yy:        driver.m_requestBodyInMemoryLimit.m_value = atoi($1.c_str());

m_requestBodyLimit is set from CONFIG_DIR_REQ_BODY_LIMIT

    | CONFIG_DIR_REQ_BODY_LIMIT
      {
        driver.m_requestBodyLimit.m_set = true;
        driver.m_requestBodyLimit.m_value = atoi($1.c_str());
      }

which in the scanner is SecRequestBodyLimit not SecRequestBodyInMemoryLimit

CONFIG_DIR_REQ_BODY_LIMIT               (?i:SecRequestBodyLimit)

From the documentation

SecRequestBodyInMemoryLimit

Description: Configures the maximum request body size that ModSecurity will store in memory.

Syntax: SecRequestBodyInMemoryLimit LIMIT_IN_BYTES 

Example Usage: SecRequestBodyInMemoryLimit 131072 

Scope: Any

Version: 2.0.0

Supported on libModSecurity: Yes

Default: 131072 (128 KB)

When a multipart/form-data request is being processed, once the in-memory limit is reached, the request body will start to be streamed into a temporary file on disk.

and

The third directive that deals with buffering, SecRequestBodyInMemoryLimit, controls how much of a request body will be stored in RAM, but it only works
with file upload (multipart/form-data) requests:

# Store up to 128 KB of request body data in memory. When
# the multipart parser reaches this limit, it will start
# using your hard disk for storage. That is generally slow,
# but unavoidable.
SecRequestBodyInMemoryLimit 131072
The request bodies that fit within the limit configured with SecRequestBodyInMemoryLimit will be stored in RAM. 
The request bodies that are larger will be streamed to disk. This directive allows you to trade performance (storing request bodies in RAM is fast) 
for size (the storage capacity of your hard disk is much bigger than that of your RAM).

I had assumed that this would have put an upper limit to the size of the buffer created in Transaction::requestBodyFromFile() that is used to read the request from file.

Is there any reason that the Transaction::requestBodyFromFile() needs to read all of the request file into memory at once? Could it read portions of the file and call Transaction::appendRequestBody() in a loop?

[AndrewFromMelbourne]

Hi @zimmerle have you had a chance to read my reply?

[zimmerle]

Hi @AndrewFromMelbourne,

Sorry for the delay. Indeed, SecRequestBodyInMemoryLimit was not being used. I've changed the parser to state that this configuration is no longer available. Here goes the reason:

LibModSecurity is able to deal with request body in a file or in a buffer (chunked or not). Nginx has this property client_body_buffer_size which controls whenever a request should be saved to a file or used as a buffer. If it is a file, ModSecurity will use the file to perform the inspection. If not, the buffer will be used.

[AndrewFromMelbourne]

Thanks very much @zimmerle!