Stricter chunked-encoding error detection

The only acceptable inputs for an HTTP chunk length/header line are
non-negative hexadecimal integers followed immediately by EOL, or followed
by `;`, then followed by chunk extensions which we ignore.

We should prevent anything other than these from being tacitly accepted as
equivalent to a length of 0, which indicates the last chunk.

Improvements in the error handling of chunked Transfer-Encoding responses
were discussed in https://gitlab.com/openconnect/openconnect/-/issues/597.

Signed-off-by: Daniel Lenski <dlenski@gmail.com>

diff --git a/http.c b/http.c
index f7435707516833d2b1b91b563e8da2015e337319..6c671a0bdc78ee0db7487db9d21a7756f7422a62 100644 (file)
--- a/http.c
+++ b/http.c
@@ -422,6 +422,7 @@ int process_http_response(struct openconnect_info *vpninfo, int connect,
                char clen_buf[16];
                /* ... else, chunked */
                while ((i = vpninfo->ssl_gets(vpninfo, clen_buf, sizeof(clen_buf)))) {
+                       char *endp = NULL;
                        int lastchunk = 0;
                        long chunklen;
 
@@ -431,8 +432,16 @@ int process_http_response(struct openconnect_info *vpninfo, int connect,
                                ret = i;
                                goto err;
                        }
-                       chunklen = strtol(clen_buf, NULL, 16);
+                       chunklen = strtol(clen_buf, &endp, 16);
+                       if (endp == clen_buf || (*endp && *endp != ';')) {
+                               /* XX: Anything other than a non-negative hex integer followed by EOL or ';' is an error. */
+                               vpn_progress(vpninfo, PRG_ERR,
+                                            _("Error in chunked decoding. Expected hexadecimal chunk length, got: '%s'\n"),
+                                            clen_buf);
+                               goto err;
+                       }
                        if (!chunklen) {
+                               /* Zero indicates the last chunk */
                                lastchunk = 1;
                                goto skip;
                        }