diff --git a/CHANGES b/CHANGES index bc76b1cf6d..1cfe48002f 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,10 @@ 6230. [bug] Prevent an unnecessary query restart if a synthesized CNAME target points to the CNAME owner. [GL #3835] +6227. [bug] Check the statistics-channel HTTP Content-length + to prevent negative or overflowing values from + causing a crash. [GL #4125] + 6224. [bug] Check the If-Modified-Since value length to prevent out-of-bounds write. [GL #4124] diff --git a/bin/tests/system/statschannel/tests.sh b/bin/tests/system/statschannel/tests.sh index 3318f45b0c..25122e1eca 100644 --- a/bin/tests/system/statschannel/tests.sh +++ b/bin/tests/system/statschannel/tests.sh @@ -72,9 +72,66 @@ loadkeys_on() { wait_for_log 20 "next key event" ns${nsidx}/named.run } +set -x + +# verify that the http server dropped the connection without replying +check_http_dropped() { + if [ -x "${NC}" ] ; then + "${NC}" 10.53.0.3 "${EXTRAPORT1}" > nc.out$n || ret=1 + if test -s nc.out$n; then + ret=1 + fi + else + echo_i "skipping test as nc not found" + fi +} + status=0 n=1 +echo_i "check content-length parse error ($n)" +ret=0 +check_http_dropped <flags = 0; - ssize_t content_len = 0; + size_t content_len = 0; bool keep_alive = false; bool host_header = false; @@ -438,12 +438,23 @@ process_request(isc_httpd_t *httpd, size_t last_len) { if (name_match(header, "Content-Length")) { char *endptr; - content_len = (size_t)strtoul(header->value, &endptr, - 10); - /* Consistency check, if we consumed all numbers */ + long val = strtol(header->value, &endptr, 10); + + errno = 0; + + /* ensure we consumed all digits */ if ((header->value + header->value_len) != endptr) { + return (ISC_R_BADNUMBER); + } + /* ensure there was no minus sign */ + if (val < 0) { + return (ISC_R_BADNUMBER); + } + /* ensure it did not overflow */ + if (errno != 0) { return (ISC_R_RANGE); } + content_len = val; } else if (name_match(header, "Connection")) { /* multiple fields in a connection header are allowed */ if (value_match(header, "close")) { @@ -479,17 +490,18 @@ process_request(isc_httpd_t *httpd, size_t last_len) { return (ISC_R_BADNUMBER); } - if (content_len == (ssize_t)ULONG_MAX) { - /* Invalid number in the header value. */ - return (ISC_R_BADNUMBER); + if (content_len >= HTTP_MAX_REQUEST_LEN) { + return (ISC_R_RANGE); } - if (httpd->consume + content_len > httpd->recvlen) { + + size_t consume = httpd->consume + content_len; + if (consume > httpd->recvlen) { /* The request data isn't complete yet. */ return (ISC_R_NOMORE); } /* Consume the request's data, which we do not use. */ - httpd->consume += content_len; + httpd->consume = consume; switch (httpd->minor_version) { case 0: