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.
This commit is contained in:
Artem Boldariev 2021-03-31 13:23:59 +03:00
parent 11ed7aac5d
commit fa062162a7

View file

@ -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);
}