file uploads over 8k fail when using ModSecurity 2.7.5 and Nginx 1.4.2

[wellumies]

Linux debian 3.2.0-4-amd64 #1 SMP Debian 3.2.46-1+deb7u1 x86_64 GNU/Linux

ModSec 2.7.5 and Nginx 1.4.2

I have an Apache backend and it receives my file uploads and requests if the file is below 8k. Only got the basic modsecurity.conf loaded without any rules. If I set the SecRequestBodyAccess = Off even those pass through. Succesful upload:

---

[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Initialising transaction (txid AcAcAGl3AcAcAcAcAcAcAcAc).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Transaction context created (dcfg 7f35a9f41980).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Starting phase REQUEST_HEADERS.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Second phase starting (dcfg 7f35a9f41980).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Input filter: Reading request body.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Multipart: Created temporary file 1 (mode 0600): /var/log/modsecurity_workdir/20130912-151049-AcAcAGl3AcAcAcAcAcAcAcAc-file-vIn5DC
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][5] Adding request argument (BODY): name "submit", value "Submit"
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Request body no files length: 150
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Input filter: Completed receiving request body (length 4719).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Starting phase REQUEST_BODY.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Hook insert_filter: Adding input forwarding filter (r 7f35a9d950a0).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Hook insert_filter: Adding output filter (r 7f35a9d950a0).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Input filter: Forwarding input: mode=0, block=0, nbytes=-1 (f 7f35a9d962b0, r 7f35a9d950a0).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Input filter: Forwarded 4719 bytes.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Input filter: Sent EOS.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Input filter: Input forwarding complete.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Starting phase RESPONSE_HEADERS.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Output filter: Not buffering response body for unconfigured MIME type "text/html".
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Output filter: Completed receiving response body (non-buffering).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Starting phase RESPONSE_BODY.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Output filter: Output forwarding complete.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Initialising logging.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Starting phase LOGGING.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Recording persistent data took 0 microseconds.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Audit log: Ignoring a non-relevant request.
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Multipart: Cleanup started (remove files 1).
[12/Sep/2013:15:10:49 +0300] [/sid#7f35a9f410a0][rid#7f35a9d950a0][/upload_file.php][4] Multipart: Deleted file (part) "/var/log/modsecurity_workdir/20130912-151049-AcAcAGl3AcAcAcAcAcAcAcAc-file-vIn5DC"

---

failed upload:

---

[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Initialising transaction (txid AcAcATAcccAcAcRcvYAIpcAc).
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Transaction context created (dcfg 7f35a9f41980).
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Starting phase REQUEST_HEADERS.
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Second phase starting (dcfg 7f35a9f41980).
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Input filter: Reading request body.
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Multipart: Created temporary file 1 (mode 0600): /var/log/modsecurity_workdir/20130912-151248-AcAcATAcccAcAcRcvYAIpcAc-file-qmZcxo
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][5] Adding request argument (BODY): name "submit", value "Submit"
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Request body no files length: 149
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Input filter: Completed receiving request body (length 8893).
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Starting phase REQUEST_BODY.
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Hook insert_filter: Adding input forwarding filter (r 7f35a9d8d0a0).
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Hook insert_filter: Adding output filter (r 7f35a9d8d0a0).
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Input filter: Forwarding input: mode=0, block=0, nbytes=-1 (f 7f35a9d8e2b0, r 7f35a9d8d0a0).
[12/Sep/2013:15:12:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Input filter: Forwarded 8192 bytes.
[12/Sep/2013:15:13:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Initialising logging.
[12/Sep/2013:15:13:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Starting phase LOGGING.
[12/Sep/2013:15:13:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Recording persistent data took 0 microseconds.
[12/Sep/2013:15:13:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Audit log: Ignoring a non-relevant request.
[12/Sep/2013:15:13:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Multipart: Cleanup started (remove files 1).
[12/Sep/2013:15:13:48 +0300] [/sid#7f35a9f410a0][rid#7f35a9d8d0a0][/upload_file.php][4] Multipart: Deleted file (part) "/var/log/modsecurity_workdir/20130912-151248-AcAcATAcccAcAcRcvYAIpcAc-file-qmZcxo"

[wellumies]

After the 4k upload ModSec adds a double charset-header with random value after UTF-8. Random value changes if I change request parameters

Content-Type=text/html; charset=UTF-8; charset=UTF-8Q

[wellumies]

There seems to be no workaround for the double charset. ModSec will add it even if nginx is configured with "charset off" or "override charset off". When ModSec sees Content-Type it will add a charset header. The memory corruption after UTF-8 is a result of unterminated string being used.

We have made modifications to the source code so at least 64k uploads now pass ModSec, but we are still testing

[wellumies]

BUG: Garbage at the end of content_type header.
REASON: Ngx_snprintf does not terminate the string. Neither does
modsecurity.
PROPOSED FIX:
diff --git a/nginx/modsecurity/ngx_http_modsecurity.c
b/nginx/modsecurity/ngx_http_modsecurity.c
index 26112a6..c1350c0 100644
--- a/nginx/modsecurity/ngx_http_modsecurity.c
+++ b/nginx/modsecurity/ngx_http_modsecurity.c
@@ -618,6 +618,7 @@
ngx_http_modsecurity_load_headers_out(ngx_http_request_t *r)
"%V; charset=%V",
&r->headers_out.content_type,
&r->headers_out.charset);

•    content_type[content_type_len-1] = 0; // ngx_snprintf does not
  

  terminate the string

    r->headers_out.content_type.data = content_type;
    r->headers_out.content_type.len = content_type_len;
  

BUG: Requests in excess of 8k are not forwarded.
REASON: It seems only the first chunk is forwarded--there's no logic to
forward the rest of the chunks.
PROPOSED FIX:
diff --git a/apache2/apache2_io.c b/apache2/apache2_io.c
index 88f1903..6629c4a 100644
--- a/apache2/apache2_io.c
+++ b/apache2/apache2_io.c
@@ -66,6 +66,7 @@ apr_status_t input_filter(ap_filter_t *f,
apr_bucket_brigade *bb_out,
" (f %pp, r %pp).", mode, block, nbytes, f, f->r);
}

+again:
if (msr->if_started_forwarding == 0) {
msr->if_started_forwarding = 1;
rc = modsecurity_request_body_retrieve_start(msr, &my_error_msg);
@@ -154,6 +155,8 @@ apr_status_t input_filter(ap_filter_t *f,
apr_bucket_brigade *bb_out,
if (msr->txcfg->debuglog_level >= 4) {
msr_log(msr, 4, "Input filter: Input forwarding complete.");
}

• } else {

•    goto again;
  

  }

  return APR_SUCCESS;

[zimmerle]

Hi @wellumies,

Indeed, the ngx_snprintf does not place the string terminator. Note that the size was allocated considering the terminator but it was not set, so It could be fixed by placing a '\0' in the end of the string. As I did here: zimmerle@ff19dcd

[wellumies]

Any comment on the second bug where file chunks are not passed to backend?

[zimmerle]

Hi @wellumies,

It seems to me that the second bug was a consequence of the first one. Can you check it again, and, if still happens, can you give more information on how to reproduce?

Thanks,
F.

[wellumies]

Can't imagine how the second bug would be a consequence of a string terminator missing, since the second bug is happening because there is no logic to pass more than 1 chunk to the backend...

Can you give me the exact git clone command to get the fixed version? I'm not familiar with Git usage

[zimmerle]

You have to clone the repository and then switch to the "trunk" branch, using:

git clone https://github.com/SpiderLabs/ModSecurity.git
git checkout origin/remotes/trunk -b trunk

[taleodor]

I have same problem with the latest trunk source code on Nginx. I'm getting error for longer (approximately, more than 64 KB) JSON POST payloads. Short payloads work fine. It also works with SecRequestBodyAccess and SecResponseBodyAccess set to Off.

[wellumies]

Haven't had time to test the header fix, but I would like to know if the file upload has been fixed? I think I'm going to lose a customer :( They have been waiting for a month now for ModSec for Nginx which is totally unusable atm, since no files can be passed to the backend.

[monasor28]

Has this issue been addressed, i still cant get over 5K+ uploads with modsec on nginx !

@wellumies: have you tried older version's ?