http2: safer invocation of populate_binsettings

populate_binsettings now returns a negative value on error, instead of a
huge positive value. Both places which call this function have been
updated to handle this change in its contract.

The way populate_binsettings had been used prior to this change the huge
positive values -- due to signed->unsigned conversion of the potentially
negative result of nghttp2_pack_settings_payload which returns negative
values on error -- are not possible. But only because http2.c currently
always provides a large enough output buffer and provides H2 SETTINGS
IVs which pass the verification logic inside nghttp2.  If the
verification logic were to change or if http2.c started passing in more
IVs without increasing the output buffer size, the overflow could become
reachable, and libcurl/curl might start leaking memory contents to
servers/proxies...

Closes #12101

diff --git a/lib/http2.c b/lib/http2.c
index c8b059498f0177176c12a6cda05c3fdb8e204147..f12f17e2b76658ad2626f445459664ff498237ee 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -107,14 +107,14 @@ static int populate_settings(nghttp2_settings_entry *iv,
   return 3;
 }
 
-static size_t populate_binsettings(uint8_t *binsettings,
-                                   struct Curl_easy *data)
+static ssize_t populate_binsettings(uint8_t *binsettings,
+                                    struct Curl_easy *data)
 {
   nghttp2_settings_entry iv[H2_SETTINGS_IV_LEN];
   int ivlen;
 
   ivlen = populate_settings(iv, data);
-  /* this returns number of bytes it wrote */
+  /* this returns number of bytes it wrote or a negative number on error. */
   return nghttp2_pack_settings_payload(binsettings, H2_BINSETTINGS_LEN,
                                        iv, ivlen);
 }
@@ -452,9 +452,14 @@ static CURLcode cf_h2_ctx_init(struct Curl_cfilter *cf,
      * in the H1 request and we upgrade from there. This stream
      * is opened implicitly as #1. */
     uint8_t binsettings[H2_BINSETTINGS_LEN];
-    size_t  binlen; /* length of the binsettings data */
+    ssize_t binlen; /* length of the binsettings data */
 
     binlen = populate_binsettings(binsettings, data);
+    if(binlen <= 0) {
+      failf(data, "nghttp2 unexpectedly failed on pack_settings_payload");
+      result = CURLE_FAILED_INIT;
+      goto out;
+    }
 
     result = http2_data_setup(cf, data, &stream);
     if(result)
@@ -1621,7 +1626,7 @@ CURLcode Curl_http2_request_upgrade(struct dynbuf *req,
   size_t blen;
   struct SingleRequest *k = &data->req;
   uint8_t binsettings[H2_BINSETTINGS_LEN];
-  size_t  binlen; /* length of the binsettings data */
+  ssize_t binlen; /* length of the binsettings data */
 
   binlen = populate_binsettings(binsettings, data);
   if(binlen <= 0) {