Fix buffer overflow with chunked HTTP handling (CVE-2019-16239)

Over a decade ago, I was vocally sad about the fact that I needed to
implement HTTP client code for myself because none of the available
options at the time gave me sufficient control over the underlying
TLS connection.

This is why. A malicious HTTP server (after we have accepted its
identity certificate) can provide bogus chunk lengths for chunked
HTTP encoding and cause a heap overflow.

Reported by Lukas Kupczyk of the Advanced Research Team at CrowdStrike
Intelligence.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/http.c b/http.c
index 51f6e7c27d5431e5e756b2e6c23409e2678aa34f..dc223580f462f75701694bbe1ba81fc1a79755e0 100644 (file)
--- a/http.c
+++ b/http.c
@@ -544,7 +544,8 @@ int process_http_response(struct openconnect_info *vpninfo, int connect,
        } else if (bodylen == BODY_CHUNKED) {
                /* ... else, chunked */
                while ((i = vpninfo->ssl_gets(vpninfo, buf, sizeof(buf)))) {
-                       int chunklen, lastchunk = 0;
+                       int lastchunk = 0;
+                       long chunklen;
 
                        if (i < 0) {
                                vpn_progress(vpninfo, PRG_ERR,
@@ -557,6 +558,18 @@ int process_http_response(struct openconnect_info *vpninfo, int connect,
                                lastchunk = 1;
                                goto skip;
                        }
+                       if (chunklen < 0) {
+                               vpn_progress(vpninfo, PRG_ERR,
+                                            _("HTTP chunk length is negative (%ld)\n"), chunklen);
+                               openconnect_close_https(vpninfo, 0);
+                               return -EINVAL;
+                       }
+                       if (chunklen >= INT_MAX) {
+                               vpn_progress(vpninfo, PRG_ERR,
+                                            _("HTTP chunk length is too large (%ld)\n"), chunklen);
+                               openconnect_close_https(vpninfo, 0);
+                               return -EINVAL;
+                       }
                        if (buf_ensure_space(body, chunklen + 1)) {
                                openconnect_close_https(vpninfo, 0);
                                return buf_error(body);

diff --git a/www/changelog.xml b/www/changelog.xml
index 09b2ae8d6c70cb18293f02c81e34162d3fdd2841..9634f4ab89c34888f5794b4d610fca256c97b817 100644 (file)
--- a/www/changelog.xml
+++ b/www/changelog.xml
@@ -16,6 +16,7 @@
    <li><b>OpenConnect HEAD</b>
      <ul>
        <li>Fix GlobalProtect ESP stall (<a href="https://gitlab.com/openconnect/openconnect/merge_requests/55">#55</a>).</li>
+       <li>Fix HTTP chunked encoding buffer overflow (CVE-2019-16239).</li>
      </ul><br/>
   </li>
   <li><b><a href="ftp://ftp.infradead.org/pub/openconnect/openconnect-8.04.tar.gz">OpenConnect v8.04</a></b>