Index: CHANGES =================================================================== --- CHANGES (revision 1757374) +++ CHANGES (working copy) @@ -1,3 +1,6 @@ *) core: CVE-2016-5387: Mitigate [f]cgi "httpoxy" issues. [Dominic Scheirlinck , Yann Ylavic] + *) core: Enforce LimitRequestFieldSize after multiple headers with the same + name have been merged. [Stefan Fritsch] + --- server/protocol.c.applied-r892678 2016-08-22 14:35:19.471028986 -0500 +++ server/protocol.c 2016-08-23 10:18:48.410459160 -0500 @@ -617,6 +617,12 @@ } } while ((len <= 0) && (++num_blank_lines < max_blank_lines)); +#ifdef AP_DEBUG_THE_REQUEST + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, + "Request received from client: %s", + ap_escape_logitem(r->pool, r->the_request)); +#endif + r->request_time = apr_time_now(); ll = r->the_request; r->method = ap_getword_white(r->pool, &ll); @@ -660,6 +666,25 @@ return 1; } +static int table_do_fn_check_lengths(void *r_, const char *key, + const char *value) +{ + request_rec *r = r_; + if (value == NULL || r->server->limit_req_fieldsize >= strlen(value) ) + return 1; + + r->status = HTTP_BAD_REQUEST; + apr_table_setn(r->notes, "error-notes", + apr_pstrcat(r->pool, "Size of a request header field " + "after merging exceeds server limit.
" + "\n
\n",
+                               ap_escape_html(r->pool, key),
+                               "
\n", NULL)); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Request header exceeds " + "LimitRequestFieldSize after merging: %s", key); + return 0; +} + /* get the length of the field name for logging, but no more than 80 bytes */ #define LOG_NAME_MAX_LEN 80 static int field_name_len(const char *field) @@ -707,19 +732,29 @@ * finding the end-of-line. This is only going to happen if it * exceeds the configured limit for a field size. */ - if (rv == APR_ENOSPC && field) { - /* ensure ap_escape_html will terminate correctly */ - field[len - 1] = '\0'; + if (rv == APR_ENOSPC) { + const char *field_escaped; + if (field && len) { + /* ensure ap_escape_html will terminate correctly */ + field[len - 1] = '\0'; + field_escaped = ap_escape_html(r->pool, field); + } + else { + field_escaped = field = ""; + } + apr_table_setn(r->notes, "error-notes", apr_psprintf(r->pool, "Size of a request header field " "exceeds server limit.
\n" - "
\n%.*s\n
/n", - field_name_len(field), - ap_escape_html(r->pool, field))); + "
\n%.*s\n
\n", + field_name_len(field_escaped), + field_escaped)); ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, - "Request header exceeds LimitRequestFieldSize: " - "%.*s", field_name_len(field), field); + "Request header exceeds LimitRequestFieldSize%s" + "%.*s", + *field ? ": " : "", + field_name_len(field), field); } return; } @@ -735,10 +770,13 @@ apr_size_t fold_len = last_len + len + 1; /* trailing null */ if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) { + const char *field_escaped; + r->status = HTTP_BAD_REQUEST; /* report what we have accumulated so far before the * overflow (last_field) as the field with the problem */ + field_escaped = ap_escape_html(r->pool, last_field); apr_table_setn(r->notes, "error-notes", apr_psprintf(r->pool, "Size of a request header field " @@ -776,6 +814,9 @@ apr_table_setn(r->notes, "error-notes", "The number of request header fields " "exceeds this server's limit."); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "Number of request headers exceeds " + "LimitRequestFields"); return; } @@ -789,7 +830,7 @@ (int)LOG_NAME_MAX_LEN, ap_escape_html(r->pool, last_field))); - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Request header field is missing ':' " "separator: %.*s", (int)LOG_NAME_MAX_LEN, last_field); @@ -849,6 +890,9 @@ * field-name, following RFC 2616, 4.2. */ apr_table_compress(r->headers_in, APR_OVERLAP_TABLES_MERGE); + + /* enforce LimitRequestFieldSize for merged headers */ + apr_table_do(table_do_fn_check_lengths, r, r->headers_in, NULL); } AP_DECLARE(void) ap_get_mime_headers(request_rec *r) @@ -915,13 +962,14 @@ if (!read_request_line(r, tmp_bb)) { if (r->status == HTTP_REQUEST_URI_TOO_LARGE || r->status == HTTP_BAD_REQUEST) { - if (r->status == HTTP_BAD_REQUEST) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "request failed: invalid characters in URI"); + if (r->status == HTTP_REQUEST_URI_TOO_LARGE) { + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "request failed: client's request-line exceeds LimitRequestLine (longer than %d)", + r->server->limit_req_line); } - else { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "request failed: URI too long (longer than %d)", r->server->limit_req_line); + else if (r->method == NULL) { + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "request failed: invalid characters in URI"); } ap_send_error_response(r, 0); ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); @@ -958,7 +1006,7 @@ ap_get_mime_headers_core(r, tmp_bb); if (r->status != HTTP_OK) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "request failed: error reading the headers"); ap_send_error_response(r, 0); ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); @@ -1005,7 +1053,7 @@ * headers! Have to dink things just to make sure the error message * comes through... */ - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "client sent invalid HTTP/0.9 request: HEAD %s", r->uri); r->header_only = 0; @@ -1047,7 +1095,7 @@ * a Host: header, and the server MUST respond with 400 if it doesn't. */ r->status = HTTP_BAD_REQUEST; - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "client sent HTTP/1.1 request without hostname " "(see RFC2616 section 14.23): %s", r->uri); } @@ -1269,7 +1317,7 @@ if (strcasecmp(ap_getword(r->pool, &auth_line, ' '), "Basic")) { /* Client tried to authenticate using wrong auth scheme */ - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "client used wrong authentication scheme: %s", r->uri); ap_note_basic_auth_failure(r); return HTTP_UNAUTHORIZED; --- server/protocol.c.applied-r892678 2016-08-23 10:23:22.057250193 -0500 +++ server/protocol.c 2016-08-23 10:34:25.926298467 -0500 @@ -617,6 +617,12 @@ } } while ((len <= 0) && (++num_blank_lines < max_blank_lines)); +#ifdef AP_DEBUG_THE_REQUEST + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, + "Request received from client: %s", + ap_escape_logitem(r->pool, r->the_request)); +#endif + r->request_time = apr_time_now(); ll = r->the_request; r->method = ap_getword_white(r->pool, &ll); @@ -660,6 +666,25 @@ return 1; } +static int table_do_fn_check_lengths(void *r_, const char *key, + const char *value) +{ + request_rec *r = r_; + if (value == NULL || r->server->limit_req_fieldsize >= strlen(value) ) + return 1; + + r->status = HTTP_BAD_REQUEST; + apr_table_setn(r->notes, "error-notes", + apr_pstrcat(r->pool, "Size of a request header field " + "after merging exceeds server limit.
" + "\n
\n",
+                               ap_escape_html(r->pool, key),
+                               "
\n", NULL)); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Request header exceeds " + "LimitRequestFieldSize after merging: %s", key); + return 0; +} + /* get the length of the field name for logging, but no more than 80 bytes */ #define LOG_NAME_MAX_LEN 80 static int field_name_len(const char *field) @@ -707,19 +732,29 @@ * finding the end-of-line. This is only going to happen if it * exceeds the configured limit for a field size. */ - if (rv == APR_ENOSPC && field) { - /* ensure ap_escape_html will terminate correctly */ - field[len - 1] = '\0'; + if (rv == APR_ENOSPC) { + const char *field_escaped; + if (field && len) { + /* ensure ap_escape_html will terminate correctly */ + field[len - 1] = '\0'; + field_escaped = ap_escape_html(r->pool, field); + } + else { + field_escaped = field = ""; + } + apr_table_setn(r->notes, "error-notes", apr_psprintf(r->pool, "Size of a request header field " "exceeds server limit.
\n" - "
\n%.*s\n
/n", - field_name_len(field), - ap_escape_html(r->pool, field))); + "
\n%.*s\n
\n", + field_name_len(field_escaped), + field_escaped)); ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, - "Request header exceeds LimitRequestFieldSize: " - "%.*s", field_name_len(field), field); + "Request header exceeds LimitRequestFieldSize%s" + "%.*s", + *field ? ": " : "", + field_name_len(field), field); } return; } @@ -735,18 +770,21 @@ apr_size_t fold_len = last_len + len + 1; /* trailing null */ if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) { + const char *field_escaped; + r->status = HTTP_BAD_REQUEST; /* report what we have accumulated so far before the * overflow (last_field) as the field with the problem */ + field_escaped = ap_escape_html(r->pool, last_field); apr_table_setn(r->notes, "error-notes", apr_psprintf(r->pool, "Size of a request header field " "after folding " "exceeds server limit.
\n" "
\n%.*s\n
\n", - field_name_len(last_field), - ap_escape_html(r->pool, last_field))); + field_name_len(field_escaped), + field_escaped)); ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Request header exceeds LimitRequestFieldSize " "after folding: %.*s", @@ -776,6 +814,9 @@ apr_table_setn(r->notes, "error-notes", "The number of request header fields " "exceeds this server's limit."); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "Number of request headers exceeds " + "LimitRequestFields"); return; } @@ -789,7 +830,7 @@ (int)LOG_NAME_MAX_LEN, ap_escape_html(r->pool, last_field))); - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "Request header field is missing ':' " "separator: %.*s", (int)LOG_NAME_MAX_LEN, last_field); @@ -849,6 +890,9 @@ * field-name, following RFC 2616, 4.2. */ apr_table_compress(r->headers_in, APR_OVERLAP_TABLES_MERGE); + + /* enforce LimitRequestFieldSize for merged headers */ + apr_table_do(table_do_fn_check_lengths, r, r->headers_in, NULL); } AP_DECLARE(void) ap_get_mime_headers(request_rec *r) @@ -918,13 +962,14 @@ if (!read_request_line(r, tmp_bb)) { if (r->status == HTTP_REQUEST_URI_TOO_LARGE || r->status == HTTP_BAD_REQUEST) { - if (r->status == HTTP_BAD_REQUEST) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "request failed: invalid characters in URI"); + if (r->status == HTTP_REQUEST_URI_TOO_LARGE) { + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "request failed: client's request-line exceeds LimitRequestLine (longer than %d)", + r->server->limit_req_line); } - else { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "request failed: URI too long (longer than %d)", r->server->limit_req_line); + else if (r->method == NULL) { + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "request failed: invalid characters in URI"); } ap_send_error_response(r, 0); ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); @@ -961,7 +1006,7 @@ ap_get_mime_headers_core(r, tmp_bb); if (r->status != HTTP_OK) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "request failed: error reading the headers"); ap_send_error_response(r, 0); ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); @@ -1008,7 +1053,7 @@ * headers! Have to dink things just to make sure the error message * comes through... */ - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "client sent invalid HTTP/0.9 request: HEAD %s", r->uri); r->header_only = 0; @@ -1050,7 +1095,7 @@ * a Host: header, and the server MUST respond with 400 if it doesn't. */ r->status = HTTP_BAD_REQUEST; - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "client sent HTTP/1.1 request without hostname " "(see RFC2616 section 14.23): %s", r->uri); } @@ -1272,7 +1317,7 @@ if (strcasecmp(ap_getword(r->pool, &auth_line, ' '), "Basic")) { /* Client tried to authenticate using wrong auth scheme */ - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, "client used wrong authentication scheme: %s", r->uri); ap_note_basic_auth_failure(r); return HTTP_UNAUTHORIZED;