From b22c87ca61d18547d4baef63b1d53a0584ff4df7 Mon Sep 17 00:00:00 2001 From: Tony Finch Date: Tue, 6 Jun 2023 18:06:43 +0100 Subject: [PATCH 1/2] Fix a stack buffer overflow in the statistics channel A long timestamp in an If-Modified-Since header could overflow a fixed-size buffer. --- bin/tests/system/statschannel/tests.sh | 37 ++++++++++++++++++++++++++ lib/isc/httpd.c | 4 ++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/statschannel/tests.sh b/bin/tests/system/statschannel/tests.sh index d2ca2e0d2c..980bdf7579 100644 --- a/bin/tests/system/statschannel/tests.sh +++ b/bin/tests/system/statschannel/tests.sh @@ -74,8 +74,23 @@ loadkeys_on() { status=0 n=1 + +echo_i "Prepare for if-modified-since test ($n)" ret=0 +i=0 +if $FEATURETEST --have-libxml2 && [ -x "${CURL}" ] ; then + URL="http://10.53.0.3:${EXTRAPORT1}/bind9.xsl" + ${CURL} --silent --show-error --fail --output bind9.xsl.1 $URL + ret=$? +else + echo_i "skipping test: requires libxml2 and curl" +fi +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) +n=$((n + 1)) + echo_i "checking consistency between named.stats and xml/json ($n)" +ret=0 rm -f ns2/named.stats $DIGCMD +tcp example ns > dig.out.$n || ret=1 $RNDCCMD 10.53.0.2 stats 2>&1 | sed 's/^/I:ns1 /' @@ -563,5 +578,27 @@ if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) n=$((n + 1)) +echo_i "Check if-modified-since works ($n)" +ret=0 +if $FEATURETEST --have-libxml2 && [ -x "${CURL}" ] ; then + URL="http://10.53.0.3:${EXTRAPORT1}/bind9.xsl" + # ensure over-long time stamps are ignored + ${CURL} --silent --show-error --fail --output bind9.xsl.2 $URL \ + --header 'If-Modified-Since: 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789' + if ! [ bind9.xsl.2 -nt bind9.xsl.1 ] || + ! ${CURL} --silent --show-error --fail \ + --output bind9.xsl.3 $URL \ + --time-cond bind9.xsl.1 || + [ -f bind9.xsl.3 ] + then + ret=1 + fi +else + echo_i "skipping test: requires libxml2 and curl" +fi +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) +n=$((n + 1)) + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 7790bcfe50..c73d7d161a 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -451,7 +451,9 @@ process_request(isc_httpd_t *httpd, size_t last_len) { if (value_match(header, "deflate")) { httpd->flags |= ACCEPT_DEFLATE; } - } else if (name_match(header, "If-Modified-Since")) { + } else if (name_match(header, "If-Modified-Since") && + header->value_len < ISC_FORMATHTTPTIMESTAMP_SIZE) + { char timestamp[ISC_FORMATHTTPTIMESTAMP_SIZE + 1]; memmove(timestamp, header->value, header->value_len); timestamp[header->value_len] = 0; From 57c8bdaff569373c8ff2e55eaf93d1f669dcfabf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 14 Aug 2023 11:20:41 +0200 Subject: [PATCH 2/2] Add CHANGES and release notes for [GL #4124] --- CHANGES | 3 +++ doc/notes/notes-current.rst | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/CHANGES b/CHANGES index 2dc43056c7..22d651b56a 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6224. [bug] Check the If-Modified-Since value length to prevent + out-of-bounds write. [GL #4124] + 6223. [func] Make -E engine option for OpenSSL Engine API use only. OpenSSL Provider API will now require engine to not be set. [GL #8153] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 25521abddd..ad69d13c21 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -37,6 +37,14 @@ Bug Fixes - None. +- The value of If-Modified-Since header in statistics channel was not checked + for length leading to possible buffer overflow by an authorized user. We + would like to emphasize that statistics channel must be properly setup to + allow access only from authorized users of the system. :gl:`#4124` + + This was reported independently by Eric Sesterhenn of X41 D-SEC and Cameron + Whitehead. + Known Issues ~~~~~~~~~~~~