From f668e723800b65113ec55643fc011c930c0cc9d5 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 22 Jan 2012 01:20:35 +0100 Subject: [PATCH] Make content_length unsigned, add overflow checks. --- http_parser.c | 59 +++++++++++++++++++++++++++++++++++++++------------ http_parser.h | 2 +- test.c | 50 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 14 deletions(-) diff --git a/http_parser.c b/http_parser.c index 17416ac..e46ca4f 100644 --- a/http_parser.c +++ b/http_parser.c @@ -27,7 +27,11 @@ #include #include #include +#include +#ifndef ULLONG_MAX +# define ULLONG_MAX ((uint64_t) -1) /* 2^64-1 */ +#endif #ifndef MIN # define MIN(a,b) ((a) < (b) ? (a) : (b)) @@ -648,7 +652,7 @@ size_t http_parser_execute (http_parser *parser, if (ch == CR || ch == LF) break; parser->flags = 0; - parser->content_length = -1; + parser->content_length = ULLONG_MAX; if (ch == 'H') { parser->state = s_res_or_resp_H; @@ -683,7 +687,7 @@ size_t http_parser_execute (http_parser *parser, case s_start_res: { parser->flags = 0; - parser->content_length = -1; + parser->content_length = ULLONG_MAX; switch (ch) { case 'H': @@ -862,7 +866,7 @@ size_t http_parser_execute (http_parser *parser, if (ch == CR || ch == LF) break; parser->flags = 0; - parser->content_length = -1; + parser->content_length = ULLONG_MAX; if (!IS_ALPHA(ch)) { SET_ERRNO(HPE_INVALID_METHOD); @@ -1427,15 +1431,29 @@ size_t http_parser_execute (http_parser *parser, break; case h_content_length: + { + uint64_t t; + if (ch == ' ') break; + if (!IS_NUM(ch)) { SET_ERRNO(HPE_INVALID_CONTENT_LENGTH); goto error; } - parser->content_length *= 10; - parser->content_length += ch - '0'; + t = parser->content_length; + t *= 10; + t += ch - '0'; + + /* Overflow? */ + if (t < parser->content_length || t == ULLONG_MAX) { + SET_ERRNO(HPE_INVALID_CONTENT_LENGTH); + goto error; + } + + parser->content_length = t; break; + } /* Transfer-Encoding: chunked */ case h_matching_transfer_encoding_chunked: @@ -1590,7 +1608,7 @@ size_t http_parser_execute (http_parser *parser, /* Content-Length header given but zero: Content-Length: 0\r\n */ parser->state = NEW_MESSAGE(); CALLBACK_NOTIFY(message_complete); - } else if (parser->content_length > 0) { + } else if (parser->content_length != ULLONG_MAX) { /* Content-Length header given and non-zero */ parser->state = s_body_identity; } else { @@ -1611,9 +1629,11 @@ size_t http_parser_execute (http_parser *parser, case s_body_identity: { - uint64_t to_read = MIN(parser->content_length, (data + len) - p); + uint64_t to_read = MIN(parser->content_length, + (uint64_t) ((data + len) - p)); - assert(parser->content_length > 0); + assert(parser->content_length != 0 + && parser->content_length != ULLONG_MAX); /* The difference between advancing content_length and p is because * the latter will automaticaly advance on the next loop iteration. @@ -1673,6 +1693,8 @@ size_t http_parser_execute (http_parser *parser, case s_chunk_size: { + uint64_t t; + assert(parser->flags & F_CHUNKED); if (ch == CR) { @@ -1692,8 +1714,17 @@ size_t http_parser_execute (http_parser *parser, goto error; } - parser->content_length *= 16; - parser->content_length += unhex_val; + t = parser->content_length; + t *= 16; + t += unhex_val; + + /* Overflow? */ + if (t < parser->content_length || t == ULLONG_MAX) { + SET_ERRNO(HPE_INVALID_CONTENT_LENGTH); + goto error; + } + + parser->content_length = t; break; } @@ -1726,10 +1757,12 @@ size_t http_parser_execute (http_parser *parser, case s_chunk_data: { - uint64_t to_read = MIN(parser->content_length, (data + len) - p); + uint64_t to_read = MIN(parser->content_length, + (uint64_t) ((data + len) - p)); assert(parser->flags & F_CHUNKED); - assert(parser->content_length > 0); + assert(parser->content_length != 0 + && parser->content_length != ULLONG_MAX); /* See the explanation in s_body_identity for why the content * length and data pointers are managed this way. @@ -1814,7 +1847,7 @@ http_message_needs_eof (http_parser *parser) return 0; } - if ((parser->flags & F_CHUNKED) || parser->content_length >= 0) { + if ((parser->flags & F_CHUNKED) || parser->content_length != ULLONG_MAX) { return 0; } diff --git a/http_parser.h b/http_parser.h index 2a8ae7d..6c9f839 100644 --- a/http_parser.h +++ b/http_parser.h @@ -209,7 +209,7 @@ struct http_parser { unsigned char index; /* index into current matcher */ uint32_t nread; /* # bytes read in various scenarios */ - int64_t content_length; /* # bytes in body (0 if no Content-Length header) */ + uint64_t content_length; /* # bytes in body (0 if no Content-Length header) */ /** READ-ONLY **/ unsigned short http_major; diff --git a/test.c b/test.c index ad5b008..05f7237 100644 --- a/test.c +++ b/test.c @@ -2003,6 +2003,53 @@ test_header_overflow_error (int req) exit(1); } +static void +test_content_length_overflow (const char *buf, size_t buflen, int expect_ok) +{ + http_parser parser; + http_parser_init(&parser, HTTP_RESPONSE); + http_parser_execute(&parser, &settings_null, buf, buflen); + + if (expect_ok) + assert(HTTP_PARSER_ERRNO(&parser) == HPE_OK); + else + assert(HTTP_PARSER_ERRNO(&parser) == HPE_INVALID_CONTENT_LENGTH); +} + +void +test_header_content_length_overflow_error (void) +{ +#define X(size) \ + "HTTP/1.1 200 OK\r\n" \ + "Content-Length: " #size "\r\n" \ + "\r\n" + const char a[] = X(18446744073709551614); /* 2^64-2 */ + const char b[] = X(18446744073709551615); /* 2^64-1 */ + const char c[] = X(18446744073709551616); /* 2^64 */ +#undef X + test_content_length_overflow(a, sizeof(a) - 1, 1); /* expect ok */ + test_content_length_overflow(b, sizeof(b) - 1, 0); /* expect failure */ + test_content_length_overflow(c, sizeof(c) - 1, 0); /* expect failure */ +} + +void +test_chunk_content_length_overflow_error (void) +{ +#define X(size) \ + "HTTP/1.1 200 OK\r\n" \ + "Transfer-Encoding: chunked\r\n" \ + "\r\n" \ + #size "\r\n" \ + "..." + const char a[] = X(FFFFFFFFFFFFFFFE); /* 2^64-2 */ + const char b[] = X(FFFFFFFFFFFFFFFF); /* 2^64-1 */ + const char c[] = X(10000000000000000); /* 2^64 */ +#undef X + test_content_length_overflow(a, sizeof(a) - 1, 1); /* expect ok */ + test_content_length_overflow(b, sizeof(b) - 1, 0); /* expect failure */ + test_content_length_overflow(c, sizeof(c) - 1, 0); /* expect failure */ +} + void test_no_overflow_long_body (int req, size_t length) { @@ -2320,6 +2367,9 @@ main (void) test_no_overflow_long_body(HTTP_RESPONSE, 1000); test_no_overflow_long_body(HTTP_RESPONSE, 100000); + test_header_content_length_overflow_error(); + test_chunk_content_length_overflow_error(); + //// RESPONSES for (i = 0; i < response_count; i++) {