Project

General

Profile

Actions
Copy link

Bug #2854

closed

chunked transfer encoding in request body only works for tiny chunks

Added by the_jk almost 7 years ago. Updated almost 7 years ago.

Status:
Fixed
Priority:
Normal
Category:
core
Target version:
1.4.49
ASK QUESTIONS IN Forums:

Description

curl -v --data-ascii "0123456789abcdef" -H "Transfer-Encoding: chunked"

fails with "400 - Bad Request" while
curl -v --data-ascii "0123456789abcde" -H "Transfer-Encoding: chunked"

succeeds as the chunk is less than 16 bytes.

Bug caused by an incorrect overflow protection in connection_handle_read_post_chunked in connections-glue.c

Files

0001-core-support-Transfer-Encoding-chunked-req-body-with.patch (2.41 KB) 0001-core-support-Transfer-Encoding-chunked-req-body-with.patch the_jk, 2018-01-10 19:53
Actions
Copy link
 #2

Updated by gstrauss almost 7 years ago

  • Status changed from New to Patch Pending
  • Target version changed from 1.4.x to 1.4.49

Ick. Thanks for reporting this bug. It is undefined behavior to right-shift a signed integral value.
Try this:

--- a/src/connections-glue.c
+++ b/src/connections-glue.c
@@ -132,7 +132,13 @@ static handler_t connection_handle_read_post_chunked(server *srv, connection *co
                 off_t hsz = p + 1 - (c->mem->ptr+c->offset);
                 unsigned char *s = (unsigned char *)c->mem->ptr+c->offset;
                 for (unsigned char u;(u=(unsigned char)hex2int(*s))!=0xFF;++s) {
-                    if (te_chunked > (~((off_t)-1) >> 4)) {
+                    static const off_t plim =
+                      (sizeof(off_t) == sizeof(uint64_t))
+                      ? (off_t)((uint64_t)~(uint64_t)0 >> 5)   /*INT64_MAX>>4*/
+                      : (sizeof(off_t) == sizeof(uint32_t))
+                        ? (off_t)((uint32_t)~(uint32_t)0 >> 5) /*INT32_MAX>>4*/
+                        : (off_t)((uint16_t)~(uint16_t)0 >> 5);/*INT16_MAX>>4*/
+                    if (te_chunked > plim) {
                         log_error_write(srv, __FILE__, __LINE__, "s",
                                         "chunked data size too large -> 400");
                         /* 400 Bad Request */

Actions
Copy link
 #3

Updated by the_jk almost 7 years ago

Right, my suggestion ended up comparing with UINT64_MAX>>4. Your code above works. But the conditionals in the above hurts my eyes ;) so, an alternative version that also work:

            (off_t)(1lu << (8 * sizeof(off_t) - 5)) - 1)

Tried to write a testcase to actually test overflow but couldn't figure out how to make the test conditional on sizeof(off_t)

Actions
Copy link
 #4

Updated by gstrauss almost 7 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100
Actions
Copy link
 #5

Updated by gstrauss almost 7 years ago

  • Has duplicate Bug #2864: Rest API failed when sending large number of data added
Actions
Copy link
 #6

Updated by gstrauss almost 7 years ago

  • Has duplicate deleted (Bug #2864: Rest API failed when sending large number of data)
Actions
Copy link

Also available in: Atom