From b7666027296a4f89a8ce6b22af335e8aee7a7782 Mon Sep 17 00:00:00 2001 From: Paul Dreik Date: Sat, 14 Sep 2019 03:16:09 +0200 Subject: doh: fix (harmless) buffer overrun Added unit test case 1655 to verify. Close #4352 the code correctly finds the flaws in the old code, if one temporarily restores doh.c to the old version. --- lib/doh.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/doh.c b/lib/doh.c index 6d1f3303b..e84c9b0ad 100644 --- a/lib/doh.c +++ b/lib/doh.c @@ -74,17 +74,26 @@ static const char *doh_strerror(DOHcode code) #define UNITTEST static #endif +/* @unittest 1655 + */ UNITTEST DOHcode doh_encode(const char *host, DNStype dnstype, unsigned char *dnsp, /* buffer */ size_t len, /* buffer size */ size_t *olen) /* output length */ { - size_t hostlen = strlen(host); + const size_t hostlen = strlen(host); unsigned char *orig = dnsp; const char *hostp = host; - if(len < (12 + hostlen + 4)) + /* The expected output length does not depend on the number of dots within + * the host name. It will always be two more than the length of the host + * name, one for the size and one trailing null. In case there are dots, + * each dot adds one size but removes the need to store the dot, net zero. + */ + const size_t expected_len = 12 + ( 1 + hostlen + 1) + 4; + + if(len < expected_len) return DOH_TOO_SMALL_BUFFER; *dnsp++ = 0; /* 16 bit id */ @@ -132,6 +141,10 @@ UNITTEST DOHcode doh_encode(const char *host, *dnsp++ = DNS_CLASS_IN; /* IN - "the Internet" */ *olen = dnsp - orig; + + /* verify that our assumption of length is valid, since + * this has lead to buffer overflows in this function */ + DEBUGASSERT(*olen == expected_len); return DOH_OK; } -- cgit v1.2.3