doh: fix leak and zero-length HTTPS RR crash

This PR fixes a leak and a crash that can happen when curl encounters
bad HTTPS RR values in DNS. We're starting to do better testing of that
kind of thing and e.g. have published bad HTTPS RR values at
dodgy.test.defo.ie.

Closes #14151

diff --git a/lib/doh.c b/lib/doh.c
index 992a11d2686ea0fe43752c10f56301aa029afa94..72b0c969d9fde6c6ff18b7ea5719d581f4b2c6a4 100644 (file)
--- a/lib/doh.c
+++ b/lib/doh.c
@@ -473,7 +473,7 @@ struct Curl_addrinfo *Curl_doh(struct Curl_easy *data,
     result = dohprobe(data, &dohp->probe[DOH_PROBE_SLOT_HTTPS],
                       DNS_TYPE_HTTPS, qname, data->set.str[STRING_DOH],
                       data->multi, dohp->headers);
-    free(qname);
+    Curl_safefree(qname);
     if(result)
       goto error;
     dohp->pending++;
@@ -1028,7 +1028,7 @@ UNITTEST void de_cleanup(struct dohentry *d)
   }
 #ifdef USE_HTTPSRR
   for(i = 0; i < d->numhttps_rrs; i++)
-    free(d->https_rrs[i].val);
+    Curl_safefree(d->https_rrs[i].val);
 #endif
 }
 
@@ -1217,18 +1217,24 @@ static CURLcode Curl_doh_decode_httpsrr(unsigned char *rrval, size_t len,
     if(pcode == HTTPS_RR_CODE_NO_DEF_ALPN)
       lhrr->no_def_alpn = TRUE;
     else if(pcode == HTTPS_RR_CODE_IPV4) {
+      if(!plen)
+        goto err;
       lhrr->ipv4hints = Curl_memdup(cp, plen);
       if(!lhrr->ipv4hints)
         goto err;
       lhrr->ipv4hints_len = (size_t)plen;
     }
     else if(pcode == HTTPS_RR_CODE_ECH) {
+      if(!plen)
+        goto err;
       lhrr->echconfiglist = Curl_memdup(cp, plen);
       if(!lhrr->echconfiglist)
         goto err;
       lhrr->echconfiglist_len = (size_t)plen;
     }
     else if(pcode == HTTPS_RR_CODE_IPV6) {
+      if(!plen)
+        goto err;
       lhrr->ipv6hints = Curl_memdup(cp, plen);
       if(!lhrr->ipv6hints)
         goto err;
@@ -1244,10 +1250,11 @@ static CURLcode Curl_doh_decode_httpsrr(unsigned char *rrval, size_t len,
   return CURLE_OK;
 err:
   if(lhrr) {
-    free(lhrr->target);
-    free(lhrr->echconfiglist);
-    free(lhrr->val);
-    free(lhrr);
+    Curl_safefree(lhrr->target);
+    Curl_safefree(lhrr->echconfiglist);
+    Curl_safefree(lhrr->val);
+    Curl_safefree(lhrr->alpns);
+    Curl_safefree(lhrr);
   }
   return CURLE_OUT_OF_MEMORY;
 }

diff --git a/tests/ech_tests.sh b/tests/ech_tests.sh
index 3862d2b211da9ddeadb135aa883c929eebfbd88d..3d9bec858aa7baaa423000f8b8fd3fe48407cb31 100755 (executable)
--- a/tests/ech_tests.sh
+++ b/tests/ech_tests.sh
@@ -68,7 +68,8 @@ declare -A ech_targets=(
     [draft-13.esni.defo.ie:12414]=""
     [crypto.cloudflare.com]="cdn-cgi/trace"
     [tls-ech.dev]=""
-    [epochbelt.com]=""
+    # this one's gone away for now (possibly temporarily)
+    # [epochbelt.com]=""
 )
 
 # Targets we expect not to be ECH-enabled servers
@@ -102,7 +103,7 @@ declare -A neither_targets=(
 : "${tout:=10s}"
 
 # Where we find OpenSSL .so's
-: "${OSSL:=$HOME/code/openssl}"
+: "${OSSL:=$HOME/code/openssl-local-inst}"
 
 # Where we find WolfSSL .so's
 : "${WSSL:=$HOME/code/wolfssl/inst/lib}"
@@ -412,6 +413,11 @@ then
             echo "Skipping $targ as ports != 443 seem blocked"
             continue
         fi
+        if [[ "$host" == "crypto.cloudflare.com" ]]
+        then
+            echo "Skipping $host as they've blocked PN override"
+            continue
+        fi
         path=${ech_targets[$targ]}
         turl="https://$host:$port/$path"
         echo "PN override check for $turl"