Skip to content
This repository has been archived by the owner on Nov 6, 2022. It is now read-only.

count the URL and headers separately when checking for HPE_HEADER_OVERFLOW errors #463

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

count the URL and headers separately when checking for HPE_HEADER_OVERFLOW errors #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3beda5f
count headers, URL and start lines separately
caiolrm Mar 16, 2019
3a5a436
make it configurable through gyp
caiolrm Mar 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions http_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <limits.h>

static uint32_t max_header_size = HTTP_MAX_HEADER_SIZE;
static uint32_t max_url_size = HTTP_MAX_URL_SIZE;

#ifndef ULLONG_MAX
# define ULLONG_MAX ((uint64_t) -1) /* 2^64-1 */
Expand Down Expand Up @@ -160,6 +161,24 @@ do { \
} \
} while (0)

/* Don't allow the total size of the request uri to exceed
* the max_url_size. If we don't check the request uri size and
* it exceeds the max_header_size, the embedder will get an
* HPE_HEADER_OVERFLOW, which is ambiguous in situations where
* what caused the overflow matters, e.g. servers deciding to
* reply either 414 or 431 status.
*/
#include <stdio.h>
#define COUNT_URL_SIZE(V, O) \
do { \
nread += (uint32_t)(V); \
url_offset = (uint8_t)(O); \
if (UNLIKELY(nread > url_offset && \
(nread - url_offset) > max_url_size)) { \
SET_ERRNO(HPE_URL_OVERFLOW); \
goto error; \
} \
} while(0) \

#define PROXY_CONNECTION "proxy-connection"
#define CONNECTION "connection"
Expand All @@ -173,12 +192,17 @@ do { \

static const char *method_strings[] =
{
#define XX(num, name, string) #string,
#define XX(num, name, string, length) #string,
HTTP_METHOD_MAP(XX)
#undef XX
};


static const uint8_t method_lengths[] =
{
#define XX(num, name, string, length) length,
HTTP_METHOD_MAP(XX)
#undef XX
};
/* Tokens as defined by rfc 2616. Also lowercases them.
* token = 1*<any CHAR except CTLs or separators>
* separators = "(" | ")" | "<" | ">" | "@"
Expand Down Expand Up @@ -358,9 +382,9 @@ enum state
, s_message_done
};


#define PARSING_HEADER(state) (state <= s_headers_done)

#define PARSING_HEADER(state) (state > s_req_http_end && state <= s_headers_done)
#define PARSING_URL(state) (state > s_req_spaces_before_url && state < s_req_http_start)
#define PARSING_START_LINE(state) (state <= s_req_spaces_before_url || (state >= s_req_http_start && state <= s_req_http_end))

enum header_states
{ h_general = 0
Expand Down Expand Up @@ -642,6 +666,8 @@ size_t http_parser_execute (http_parser *parser,
{
char c, ch;
int8_t unhex_val;
uint8_t url_offset;
uint32_t spaces_before_url = 0;
const char *p = data;
const char *header_field_mark = 0;
const char *header_value_mark = 0;
Expand Down Expand Up @@ -707,9 +733,14 @@ size_t http_parser_execute (http_parser *parser,
for (p=data; p != data + len; p++) {
ch = *p;

if (PARSING_HEADER(CURRENT_STATE()))
if (PARSING_HEADER(CURRENT_STATE())) {
COUNT_HEADER_SIZE(1);

} else if (PARSING_URL(CURRENT_STATE())) {
COUNT_URL_SIZE(1, method_lengths[parser->method] + spaces_before_url);
} else if (PARSING_START_LINE(CURRENT_STATE())) {
nread++;
}

reexecute:
switch (CURRENT_STATE()) {

Expand Down Expand Up @@ -1015,7 +1046,10 @@ size_t http_parser_execute (http_parser *parser,

case s_req_spaces_before_url:
{
if (ch == ' ') break;
if (ch == ' ') {
++spaces_before_url;
break;
}

MARK(url);
if (parser->method == HTTP_CONNECT) {
Expand Down Expand Up @@ -2499,3 +2533,8 @@ void
http_parser_set_max_header_size(uint32_t size) {
max_header_size = size;
}

void
http_parser_set_max_uri_size(uint32_t size) {
max_url_size = size;
}
14 changes: 12 additions & 2 deletions http_parser.gyp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
],
},

'variables': {
'http_max_url_size%': '8192'
},

'targets': [
{
'target_name': 'http_parser',
Expand All @@ -56,7 +60,10 @@
'defines': [ 'HTTP_PARSER_STRICT=0' ],
'include_dirs': [ '.' ],
},
'defines': [ 'HTTP_PARSER_STRICT=0' ],
'defines': [
'HTTP_MAX_URL_SIZE=<(http_max_url_size)',
'HTTP_PARSER_STRICT=0'
],
'sources': [ './http_parser.c', ],
'conditions': [
['OS=="win"', {
Expand All @@ -79,7 +86,10 @@
'defines': [ 'HTTP_PARSER_STRICT=1' ],
'include_dirs': [ '.' ],
},
'defines': [ 'HTTP_PARSER_STRICT=1' ],
'defines': [
'HTTP_MAX_URL_SIZE=<(http_max_url_size)',
'HTTP_PARSER_STRICT=1'
],
'sources': [ './http_parser.c', ],
'conditions': [
['OS=="win"', {
Expand Down
104 changes: 60 additions & 44 deletions http_parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ typedef unsigned __int64 uint64_t;
# define HTTP_MAX_HEADER_SIZE (80*1024)
#endif

/* Maximium uri size allowed. If the macro is not defined
* before including this header then the default is used. To
* change the maximum uri size, define the macro in the build
* environment (e.g. -DHTTP_MAX_URL_SIZE=<value>). To remove
* the effective limit on the size of the uri, define the macro
* to a very large number (e.g. -DHTTP_MAX_URL_SIZE=0x7fffffff)
*/
#ifndef HTTP_MAX_URL_SIZE
# define HTTP_MAX_URL_SIZE (80*1024)
#endif

typedef struct http_parser http_parser;
typedef struct http_parser_settings http_parser_settings;

Expand Down Expand Up @@ -160,53 +171,53 @@ enum http_status


/* Request Methods */
#define HTTP_METHOD_MAP(XX) \
XX(0, DELETE, DELETE) \
XX(1, GET, GET) \
XX(2, HEAD, HEAD) \
XX(3, POST, POST) \
XX(4, PUT, PUT) \
/* pathological */ \
XX(5, CONNECT, CONNECT) \
XX(6, OPTIONS, OPTIONS) \
XX(7, TRACE, TRACE) \
/* WebDAV */ \
XX(8, COPY, COPY) \
XX(9, LOCK, LOCK) \
XX(10, MKCOL, MKCOL) \
XX(11, MOVE, MOVE) \
XX(12, PROPFIND, PROPFIND) \
XX(13, PROPPATCH, PROPPATCH) \
XX(14, SEARCH, SEARCH) \
XX(15, UNLOCK, UNLOCK) \
XX(16, BIND, BIND) \
XX(17, REBIND, REBIND) \
XX(18, UNBIND, UNBIND) \
XX(19, ACL, ACL) \
/* subversion */ \
XX(20, REPORT, REPORT) \
XX(21, MKACTIVITY, MKACTIVITY) \
XX(22, CHECKOUT, CHECKOUT) \
XX(23, MERGE, MERGE) \
/* upnp */ \
XX(24, MSEARCH, M-SEARCH) \
XX(25, NOTIFY, NOTIFY) \
XX(26, SUBSCRIBE, SUBSCRIBE) \
XX(27, UNSUBSCRIBE, UNSUBSCRIBE) \
/* RFC-5789 */ \
XX(28, PATCH, PATCH) \
XX(29, PURGE, PURGE) \
/* CalDAV */ \
XX(30, MKCALENDAR, MKCALENDAR) \
/* RFC-2068, section 19.6.1.2 */ \
XX(31, LINK, LINK) \
XX(32, UNLINK, UNLINK) \
/* icecast */ \
XX(33, SOURCE, SOURCE) \
#define HTTP_METHOD_MAP(XX) \
XX(0, DELETE, DELETE, 6) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP_METHOD_MAP is a public API. Changing it makes it a semver-major change.

XX(1, GET, GET, 3) \
XX(2, HEAD, HEAD, 4) \
XX(3, POST, POST, 4) \
XX(4, PUT, PUT, 3) \
/* pathological */ \
XX(5, CONNECT, CONNECT, 7) \
XX(6, OPTIONS, OPTIONS, 7) \
XX(7, TRACE, TRACE, 5) \
/* WebDAV */ \
XX(8, COPY, COPY, 4) \
XX(9, LOCK, LOCK, 4) \
XX(10, MKCOL, MKCOL, 5) \
XX(11, MOVE, MOVE, 4) \
XX(12, PROPFIND, PROPFIND, 8) \
XX(13, PROPPATCH, PROPPATCH, 9) \
XX(14, SEARCH, SEARCH, 6) \
XX(15, UNLOCK, UNLOCK, 6) \
XX(16, BIND, BIND, 4) \
XX(17, REBIND, REBIND, 6) \
XX(18, UNBIND, UNBIND, 6) \
XX(19, ACL, ACL, 3) \
/* subversion */ \
XX(20, REPORT, REPORT, 6) \
XX(21, MKACTIVITY, MKACTIVITY, 10) \
XX(22, CHECKOUT, CHECKOUT, 8) \
XX(23, MERGE, MERGE, 5) \
/* upnp */ \
XX(24, MSEARCH, M-SEARCH, 7) \
XX(25, NOTIFY, NOTIFY, 6) \
XX(26, SUBSCRIBE, SUBSCRIBE, 9) \
XX(27, UNSUBSCRIBE, UNSUBSCRIBE, 11) \
/* RFC-5789 */ \
XX(28, PATCH, PATCH, 5) \
XX(29, PURGE, PURGE, 5) \
/* CalDAV */ \
XX(30, MKCALENDAR, MKCALENDAR, 10) \
/* RFC-2068, section 19.6.1.2 */ \
XX(31, LINK, LINK, 4) \
XX(32, UNLINK, UNLINK, 6) \
/* icecast */ \
XX(33, SOURCE, SOURCE, 6) \

enum http_method
{
#define XX(num, name, string) HTTP_##name = num,
#define XX(num, name, string, length) HTTP_##name = num,
HTTP_METHOD_MAP(XX)
#undef XX
};
Expand Down Expand Up @@ -252,6 +263,8 @@ enum flags
XX(INVALID_EOF_STATE, "stream ended at an unexpected time") \
XX(HEADER_OVERFLOW, \
"too many header bytes seen; overflow detected") \
XX(URL_OVERFLOW, \
"too many url bytes seen; overflow detected") \
XX(CLOSED_CONNECTION, \
"data received after completed connection: close message") \
XX(INVALID_VERSION, "invalid HTTP version") \
Expand Down Expand Up @@ -433,6 +446,9 @@ int http_body_is_final(const http_parser *parser);
/* Change the maximum header size provided at compile time. */
void http_parser_set_max_header_size(uint32_t size);

/* Change the maximum uri size provided at compile time. */
void http_parser_set_max_uri_size(uint32_t size);

#ifdef __cplusplus
}
#endif
Expand Down
28 changes: 28 additions & 0 deletions test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3756,6 +3756,32 @@ test_header_overflow_error (int req)
abort();
}

void
test_URL_OVERFLOW_error ()
{
http_parser parser;
http_parser_init(&parser, HTTP_REQUEST);
size_t parsed;
const char *buf;
buf = "GET /";
parsed = http_parser_execute(&parser, &settings_null, buf, strlen(buf));
assert(parsed == strlen(buf));

buf = "a";
size_t buflen = strlen(buf);

int i;
for (i = 0; i < HTTP_MAX_URL_SIZE - 1; i++) { //-1 because it started with a slash
parsed = http_parser_execute(&parser, &settings_null, buf, buflen);
if (parsed != buflen) {
assert(HTTP_PARSER_ERRNO(&parser) == HPE_URL_OVERFLOW);
return;
}
}

fprintf(stderr, "\n*** Error expected but none in uri overflow test ***\n");
abort();
}

void
test_header_nread_value ()
Expand Down Expand Up @@ -4159,6 +4185,8 @@ main (void)
//// OVERFLOW CONDITIONS
test_no_overflow_parse_url();

test_URL_OVERFLOW_error();

test_header_overflow_error(HTTP_REQUEST);
test_no_overflow_long_body(HTTP_REQUEST, 1000);
test_no_overflow_long_body(HTTP_REQUEST, 100000);
Expand Down