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

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

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>