Skip to content

Commit

Permalink
[core,security] process headers after combining folded headers
Browse files Browse the repository at this point in the history
- this fixes various use-after-free scenarios (reported by Or Peles of
  VDOO): when parse_single_header stores pointers to header values in
  con->request, those pointers are not updated if the header value is
  reallocated when folded header lines are appended.
- also remove trailing white-space from folded lines
  • Loading branch information
stbuehler committed Aug 26, 2018
1 parent a2cc330 commit df8e4f9
Showing 1 changed file with 42 additions and 40 deletions.
82 changes: 42 additions & 40 deletions src/request.c
Expand Up @@ -444,6 +444,11 @@ static void init_parse_header_state(parse_header_state* state) {
static int parse_single_header(server *srv, connection *con, parse_header_state *state, data_string *ds) {
int cmp = 0;

/* empty header-fields are not allowed by HTTP-RFC, we just ignore them */
if (buffer_string_is_empty(ds->value)) {
goto drop_header;
}

/* retreive values
*
*
Expand Down Expand Up @@ -575,6 +580,7 @@ int http_request_parse(server *srv, connection *con) {
char *uri = NULL, *proto = NULL, *method = NULL;
int is_key = 1, key_len = 0, is_ws_after_key = 0, in_folding;
char *value = NULL, *key = NULL;
data_string *current_header = NULL;

int line = 0;

Expand Down Expand Up @@ -1024,7 +1030,8 @@ int http_request_parse(server *srv, connection *con) {
case '\r':
case '\n':
if (*cur == '\n' || con->parse_request->ptr[i+1] == '\n') {
data_string *ds = NULL;
int value_len;

if (*cur == '\n') {
if (http_header_strict) {
http_request_missing_CR_before_LF(srv, con);
Expand All @@ -1038,17 +1045,15 @@ int http_request_parse(server *srv, connection *con) {
/* End of Headerline */
con->parse_request->ptr[i] = '\0';

value_len = cur - value;

/* strip trailing white-spaces */
while (value_len > 0 && (value[value_len - 1] == ' ' || value[value_len - 1] == '\t')) {
--value_len;
}

if (in_folding) {
/**
* we use a evil hack to handle the line-folding
*
* As array_insert_unique() deletes 'ds' in the case of a duplicate
* ds points somewhere and we get a evil crash. As a solution we keep the old
* "key" and get the current value from the hash and append us
*
* */

if (!key || !key_len) {
if (!current_header) {
/* 400 */

if (srv->srvconf.log_request_header_on_error) {
Expand All @@ -1062,47 +1067,32 @@ int http_request_parse(server *srv, connection *con) {
goto failure;
}

if (NULL != (ds = (data_string *)array_get_element_klen(con->request.headers, key, key_len))) {
buffer_append_string(ds->value, value);
}
buffer_append_string_len(current_header->value, value, value_len);
} else {
int s_len;
key = con->parse_request->ptr + first;

s_len = cur - value;

/* strip trailing white-spaces */
for (; s_len > 0 &&
(value[s_len - 1] == ' ' ||
value[s_len - 1] == '\t'); s_len--);

value[s_len] = '\0';

if (s_len > 0) {
if (NULL == (ds = (data_string *)array_get_unused_element(con->request.headers, TYPE_STRING))) {
ds = data_string_init();
}
buffer_copy_string_len(ds->key, key, key_len);
buffer_copy_string_len(ds->value, value, s_len);

/* process previous header */
if (current_header) {
data_string *ds = current_header;
current_header = NULL;
if (!parse_single_header(srv, con, &state, ds)) {
/* parse_single_header should already have logged it */
goto failure;
}
} else {
/* empty header-fields are not allowed by HTTP-RFC, we just ignore them */
}

key = con->parse_request->ptr + first;

if (NULL == (current_header = (data_string *)array_get_unused_element(con->request.headers, TYPE_STRING))) {
current_header = data_string_init();
}

buffer_copy_string_len(current_header->key, key, key_len);
buffer_copy_string_len(current_header->value, value, value_len);
}

first = i+1;
is_key = 1;
value = NULL;
#if 0
/**
* for Bug 1230 keep the key_len a live
*/
key_len = 0;
#endif
in_folding = 0;
} else {
if (srv->srvconf.log_request_header_on_error) {
Expand Down Expand Up @@ -1132,6 +1122,16 @@ int http_request_parse(server *srv, connection *con) {
}
}

/* process last header */
if (current_header) {
data_string* ds = current_header;
current_header = NULL;
if (!parse_single_header(srv, con, &state, ds)) {
/* parse_single_header should already have logged it */
goto failure;
}
}

con->header_len = i;

/* do some post-processing */
Expand Down Expand Up @@ -1252,6 +1252,8 @@ int http_request_parse(server *srv, connection *con) {
return 0;

failure:
if (current_header) current_header->free((data_unset *)current_header);

con->keep_alive = 0;
con->response.keep_alive = 0;
if (!con->http_status) con->http_status = 400;
Expand Down

0 comments on commit df8e4f9

Please sign in to comment.