Fix crash (regression) in DIG when handling non-DoH responses

This commit fixes crash in dig when it encounters non-expected header
value. The bug was introduced at some point late in the last DoH
development cycle. Also, refactors the relevant code a little bit to
ensure better incoming data validation for client-side DoH
connections.

diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c
index 8d0de0a7e695a38780e0c7b2dbe8b656f235ab32..5fbd0ac90f8a3ef2290909c068495f31653d9e30 100644 (file)
--- a/lib/isc/netmgr/http.c
+++ b/lib/isc/netmgr/http.c
@@ -55,6 +55,13 @@
        (((namelen) == sizeof(header) - 1) && \
         (strncasecmp((header), (const char *)(name), (namelen)) == 0))
 
+#define MIN_SUCCESSFUL_HTTP_STATUS (200)
+#define MAX_SUCCESSFUL_HTTP_STATUS (299)
+
+#define SUCCESSFUL_HTTP_STATUS(code)             \
+       ((code) >= MIN_SUCCESSFUL_HTTP_STATUS && \
+        (code) <= MAX_SUCCESSFUL_HTTP_STATUS)
+
 typedef struct isc_nm_http_response_status {
        size_t code;
        size_t content_length;
@@ -507,6 +514,19 @@ on_data_chunk_recv_callback(nghttp2_session *ngsession, uint8_t flags,
        return (rv);
 }
 
+static void
+call_unlink_cstream_readcb(http_cstream_t *cstream,
+                          isc_nm_http_session_t *session,
+                          isc_result_t result) {
+       REQUIRE(VALID_HTTP2_SESSION(session));
+       REQUIRE(cstream != NULL);
+       cstream->read_cb(session->handle, result,
+                        &(isc_region_t){ cstream->rbuf, cstream->rbufsize },
+                        cstream->read_cbarg);
+       ISC_LIST_UNLINK(session->cstreams, cstream, link);
+       put_http_cstream(session->mctx, cstream);
+}
+
 static int
 on_client_stream_close_callback(int32_t stream_id,
                                isc_nm_http_session_t *session) {
@@ -514,16 +534,10 @@ on_client_stream_close_callback(int32_t stream_id,
 
        if (cstream != NULL) {
                isc_result_t result =
-                       cstream->response_status.code >= 200 &&
-                                       cstream->response_status.code < 300
+                       SUCCESSFUL_HTTP_STATUS(cstream->response_status.code)
                                ? ISC_R_SUCCESS
                                : ISC_R_FAILURE;
-               cstream->read_cb(
-                       session->handle, result,
-                       &(isc_region_t){ cstream->rbuf, cstream->rbufsize },
-                       cstream->read_cbarg);
-               ISC_LIST_UNLINK(session->cstreams, cstream, link);
-               put_http_cstream(session->mctx, cstream);
+               call_unlink_cstream_readcb(cstream, session, result);
                if (ISC_LIST_EMPTY(session->cstreams)) {
                        int rv = 0;
                        rv = nghttp2_session_terminate_session(
@@ -576,7 +590,7 @@ on_stream_close_callback(nghttp2_session *ngsession, int32_t stream_id,
        return (rv);
 }
 
-static void
+static bool
 client_handle_status_header(http_cstream_t *cstream, const uint8_t *value,
                            const size_t valuelen) {
        char tmp[32] = { 0 };
@@ -584,9 +598,15 @@ client_handle_status_header(http_cstream_t *cstream, const uint8_t *value,
 
        strncpy(tmp, (const char *)value, ISC_MIN(tmplen, valuelen));
        cstream->response_status.code = strtoul(tmp, NULL, 10);
+
+       if (SUCCESSFUL_HTTP_STATUS(cstream->response_status.code)) {
+               return (true);
+       }
+
+       return (false);
 }
 
-static void
+static bool
 client_handle_content_length_header(http_cstream_t *cstream,
                                    const uint8_t *value,
                                    const size_t valuelen) {
@@ -595,9 +615,17 @@ client_handle_content_length_header(http_cstream_t *cstream,
 
        strncpy(tmp, (const char *)value, ISC_MIN(tmplen, valuelen));
        cstream->response_status.content_length = strtoul(tmp, NULL, 10);
+
+       if (cstream->response_status.content_length == 0 ||
+           cstream->response_status.content_length > MAX_DNS_MESSAGE_SIZE)
+       {
+               return (false);
+       }
+
+       return (true);
 }
 
-static void
+static bool
 client_handle_content_type_header(http_cstream_t *cstream, const uint8_t *value,
                                  const size_t valuelen) {
        const char type_dns_message[] = DNS_MEDIA_TYPE;
@@ -607,7 +635,10 @@ client_handle_content_type_header(http_cstream_t *cstream, const uint8_t *value,
 
        if (strncasecmp((const char *)value, type_dns_message, len) == 0) {
                cstream->response_status.content_type_valid = true;
+               return (true);
        }
+
+       return (false);
 }
 
 static int
@@ -620,6 +651,7 @@ client_on_header_callback(nghttp2_session *ngsession,
        const char status[] = ":status";
        const char content_length[] = "Content-Length";
        const char content_type[] = "Content-Type";
+       bool header_ok = true;
 
        REQUIRE(VALID_HTTP2_SESSION(session));
        REQUIRE(session->client);
@@ -637,20 +669,22 @@ client_on_header_callback(nghttp2_session *ngsession,
                }
 
                if (HEADER_MATCH(status, name, namelen)) {
-                       client_handle_status_header(cstream, value, valuelen);
+                       header_ok = client_handle_status_header(cstream, value,
+                                                               valuelen);
                } else if (HEADER_MATCH(content_length, name, namelen)) {
-                       client_handle_content_length_header(cstream, value,
-                                                           valuelen);
+                       header_ok = client_handle_content_length_header(
+                               cstream, value, valuelen);
                } else if (HEADER_MATCH(content_type, name, namelen)) {
-                       client_handle_content_type_header(cstream, value,
-                                                         valuelen);
-                       if (!cstream->response_status.content_type_valid) {
-                               return (NGHTTP2_ERR_HTTP_HEADER);
-                       }
+                       header_ok = client_handle_content_type_header(
+                               cstream, value, valuelen);
                }
                break;
        }
 
+       if (!header_ok) {
+               return (NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE);
+       }
+
        return (0);
 }