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.

1 files changed, 15 insertions, 2 deletions

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