From ac612dfeee95a014bf0a5f31f60ca7cfe9d74a21 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Tue, 27 Sep 2022 17:20:23 +0200
Subject: [PATCH] altsvc: reject bad port numbers

The existing code tried but did not properly reject alternative services
using negative or too large port numbers.

With this fix, the logic now also flushes the old entries immediately
before adding a new one, making a following header with an illegal entry
not flush the already stored entry.

Report from the ongoing source code audit by Trail of Bits.

Adjusted test 356 to verify.

Closes #9607
---
 lib/altsvc.c       | 25 ++++++++++++++-----------
 tests/data/test356 |  2 ++
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/lib/altsvc.c b/lib/altsvc.c
index bda3bf4e0a..7bca840151 100644
--- a/lib/altsvc.c
+++ b/lib/altsvc.c
@@ -462,6 +462,7 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
   struct altsvc *as;
   unsigned short dstport = srcport; /* the same by default */
   CURLcode result = getalnum(&p, alpnbuf, sizeof(alpnbuf));
+  size_t entries = 0;
 #ifdef CURL_DISABLE_VERBOSE_STRINGS
   (void)data;
 #endif
@@ -472,11 +473,10 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
 
   DEBUGASSERT(asi);
 
-  /* Flush all cached alternatives for this source origin, if any */
-  altsvc_flush(asi, srcalpnid, srchost, srcport);
-
   /* "clear" is a magic keyword */
   if(strcasecompare(alpnbuf, "clear")) {
+    /* Flush cached alternatives for this source origin */
+    altsvc_flush(asi, srcalpnid, srchost, srcport);
     return CURLE_OK;
   }
 
@@ -494,6 +494,7 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
         bool quoted = FALSE;
         time_t maxage = 24 * 3600; /* default is 24 hours */
         bool persist = FALSE;
+        bool valid = TRUE;
         p++;
         if(*p != ':') {
           /* host name starts here */
@@ -503,7 +504,7 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
           len = p - hostp;
           if(!len || (len >= MAX_ALTSVC_HOSTLEN)) {
             infof(data, "Excessive alt-svc host name, ignoring.");
-            dstalpnid = ALPN_none;
+            valid = FALSE;
           }
           else {
             memcpy(namebuf, hostp, len);
@@ -520,10 +521,11 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
           unsigned long port = strtoul(++p, &end_ptr, 10);
           if(port > USHRT_MAX || end_ptr == p || *end_ptr != '\"') {
             infof(data, "Unknown alt-svc port number, ignoring.");
-            dstalpnid = ALPN_none;
+            valid = FALSE;
           }
+          else
+            dstport = curlx_ultous(port);
           p = end_ptr;
-          dstport = curlx_ultous(port);
         }
         if(*p++ != '\"')
           break;
@@ -575,7 +577,12 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
               persist = TRUE;
           }
         }
-        if(dstalpnid) {
+        if(dstalpnid && valid) {
+          if(!entries++)
+            /* Flush cached alternatives for this source origin, if any - when
+               this is the first entry of the line. */
+            altsvc_flush(asi, srcalpnid, srchost, srcport);
+
           as = altsvc_createid(srchost, dsthost,
                                srcalpnid, dstalpnid,
                                srcport, dstport);
@@ -589,10 +596,6 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
                   Curl_alpnid2str(dstalpnid));
           }
         }
-        else {
-          infof(data, "Unknown alt-svc protocol \"%s\", skipping.",
-                alpnbuf);
-        }
       }
       else
         break;
diff --git a/tests/data/test356 b/tests/data/test356
index 0f87958997..9f6459487c 100644
--- a/tests/data/test356
+++ b/tests/data/test356
@@ -16,7 +16,9 @@ Content-Length: 6
 Connection: close
 Content-Type: text/html
 Funny-head: yesyes
+Alt-Svc: h1="nowhere.foo:-1"
 Alt-Svc: h1="nowhere.foo:81", un-kno22!wn=":82"
+Alt-Svc: h1="nowhere.foo:70000"
 
 -foo-
 </data>
-- 
2.47.3