From 0a94d182415f554d28ce6e16c6bfd2e136a522ee Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Fri, 3 May 2024 15:06:54 +0200
Subject: [PATCH] doh: cleanups in ECH related functions

- make local_decode_rdata_name use dynbuf instead of calloc + memcpy
- avoid extra memdup in local_decode_rdata_alpn
- no need to if() before free()
- use memdup instead of calloc + memcpy in Curl_doh_decode_httpsrr

Reviewed-by: Stephen Farrell
Closes #13526
---
 lib/doh.c | 56 ++++++++++++++++++++++---------------------------------
 1 file changed, 22 insertions(+), 34 deletions(-)

diff --git a/lib/doh.c b/lib/doh.c
index 22864c1b67..03317ae27e 100644
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -1045,50 +1045,45 @@ static CURLcode local_decode_rdata_name(unsigned char **buf, size_t *remaining,
 {
   unsigned char *cp = NULL;
   int rem = 0;
-  char *thename = NULL, *tp = NULL;
   unsigned char clen = 0; /* chunk len */
+  struct dynbuf thename;
 
+  DEBUGASSERT(buf && remaining && dnsname);
   if(!buf || !remaining || !dnsname)
     return CURLE_OUT_OF_MEMORY;
   rem = (int)*remaining;
-  thename = calloc(1, CURL_MAXLEN_host_name);
-  if(!thename)
+  if(rem <= 0) {
+    Curl_dyn_free(&thename);
     return CURLE_OUT_OF_MEMORY;
+  }
+  Curl_dyn_init(&thename, CURL_MAXLEN_host_name);
   cp = *buf;
-  tp = thename;
   clen = *cp++;
   if(clen == 0) {
     /* special case - return "." as name */
-    thename[0] = '.';
-    thename[1] = 0x00;
+    if(Curl_dyn_addn(&thename, ".", 1))
+      return CURLE_OUT_OF_MEMORY;
   }
   while(clen) {
     if(clen >= rem) {
-      free(thename);
-      return CURLE_OUT_OF_MEMORY;
-    }
-    if(((tp - thename) + clen) > CURL_MAXLEN_host_name) {
-      free(thename);
+      Curl_dyn_free(&thename);
       return CURLE_OUT_OF_MEMORY;
     }
-    memcpy(tp, cp, clen);
-    tp += clen;
-    *tp++ = '.';
+    if(Curl_dyn_addn(&thename, cp, clen) ||
+       Curl_dyn_addn(&thename, ".", 1))
+      return CURLE_TOO_LARGE;
+
     cp += clen;
     rem -= (clen + 1);
     if(rem <= 0) {
-      free(thename);
+      Curl_dyn_free(&thename);
       return CURLE_OUT_OF_MEMORY;
     }
     clen = *cp++;
   }
   *buf = cp;
-  if(rem <= 0) {
-    free(thename);
-    return CURLE_OUT_OF_MEMORY;
-  }
   *remaining = rem - 1;
-  *dnsname = thename;
+  *dnsname = Curl_dyn_ptr(&thename);
   return CURLE_OK;
 }
 
@@ -1108,7 +1103,7 @@ static CURLcode local_decode_rdata_alpn(unsigned char *rrval, size_t len,
    */
   int remaining = (int) len;
   char *oval;
-  size_t olen = 0, i;
+  size_t i;
   unsigned char *cp = rrval;
   struct dynbuf dval;
 
@@ -1137,13 +1132,10 @@ static CURLcode local_decode_rdata_alpn(unsigned char *rrval, size_t len,
     }
     remaining -= (int)tlen;
   }
-  olen = Curl_dyn_len(&dval);
-  /* I think the + 1 here is ok but it could trigger a read error */
-  oval = (char *)Curl_memdup(Curl_dyn_ptr(&dval), olen + 1);
+  /* this string is always null terminated */
+  oval = Curl_dyn_ptr(&dval);
   if(!oval)
     goto err;
-  Curl_dyn_free(&dval);
-  oval[olen]='\0';
   *alpns = oval;
   return CURLE_OK;
 err:
@@ -1192,11 +1184,10 @@ static CURLcode Curl_doh_decode_httpsrr(unsigned char *rrval, size_t len,
   lhrr = calloc(1, sizeof(struct Curl_https_rrinfo));
   if(!lhrr)
     return CURLE_OUT_OF_MEMORY;
-  lhrr->val = calloc(1, len);
+  lhrr->val = Curl_memdup(rrval, len);
   if(!lhrr->val)
     goto err;
   lhrr->len = len;
-  memcpy(lhrr->val, rrval, len);
   if(remaining <= 2)
     goto err;
   lhrr->priority = (uint16_t)((cp[0] << 8) + cp[1]);
@@ -1245,12 +1236,9 @@ static CURLcode Curl_doh_decode_httpsrr(unsigned char *rrval, size_t len,
   return CURLE_OK;
 err:
   if(lhrr) {
-    if(lhrr->target)
-      free(lhrr->target);
-    if(lhrr->echconfiglist)
-      free(lhrr->echconfiglist);
-    if(lhrr->val)
-      free(lhrr->val);
+    free(lhrr->target);
+    free(lhrr->echconfiglist);
+    free(lhrr->val);
     free(lhrr);
   }
   return CURLE_OUT_OF_MEMORY;
-- 
2.47.3