upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-06-08 20:12:06 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Parse statschannel Content-Length: more carefully

Browse source 
A negative or excessively large Content-Length could cause a crash
by making `INSIST(httpd->consume != 0)` fail.
This commit is contained in:
Tony Finch 2023-06-09 09:33:57 +01:00 committed by Ondřej Surý
parent 159c880240
commit 26e10e8fb5
No known key found for this signature in database
GPG key ID: 2820F37E873DEA41
2 changed files with 78 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
bin/tests/system/statschannel/tests.sh
View file

@ -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 <<EOF
POST /xml/v3/status HTTP/1.0
Content-Length: nah
EOF
if [ $ret != 0 ]; then echo_i "failed"; fi
status=$((status + ret))
n=$((n + 1))
echo_i "check negative content-length ($n)"
ret=0
check_http_dropped <<EOF
POST /xml/v3/status HTTP/1.0
Content-Length: -50
EOF
if [ $ret != 0 ]; then echo_i "failed"; fi
status=$((status + ret))
n=$((n + 1))
echo_i "check content-length 32-bit overflow ($n)"
check_http_dropped <<EOF
POST /xml/v3/status HTTP/1.0
Content-Length: 4294967239
EOF
if [ $ret != 0 ]; then echo_i "failed"; fi
status=$((status + ret))
n=$((n + 1))
echo_i "check content-length 64-bit overflow ($n)"
check_http_dropped <<EOF
POST /xml/v3/status HTTP/1.0
Content-Length: 18446744073709551549
EOF
if [ $ret != 0 ]; then echo_i "failed"; fi
status=$((status + ret))
n=$((n + 1))
echo_i "Prepare for if-modified-since test ($n)"
ret=0
i=0

30
lib/isc/httpd.c
View file

@ -420,7 +420,7 @@ process_request(isc_httpd_t *httpd, size_t last_len) {
*/
httpd->flags = 0;
ssize_t content_len = 0;
size_t content_len = 0;
bool keep_alive = false;
bool host_header = false;
@ -431,12 +431,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")) {
@ -472,17 +483,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: