Protect base64 encoding buffers (#2447)

Check bounds before base64-encoding data into fixed-size buffers.

diff --git a/lib/sspi/sspwin32.cc b/lib/sspi/sspwin32.cc
index 636731751bb997c894bc9d58c99657d35439b268..a07b5ceb3010ef456aed1491c7e488414804f06f 100644 (file)
--- a/lib/sspi/sspwin32.cc
+++ b/lib/sspi/sspwin32.cc
@@ -500,6 +500,7 @@ const char * WINAPI SSP_MakeChallenge(PVOID PNegotiateBuf, int NegotiateLen)
         struct base64_encode_ctx ctx;
         base64_encode_init(&ctx);
         static char encoded[8192];
+        assert(base64_encode_len(cbOut) < sizeof(encoded));
         size_t dstLen = base64_encode_update(&ctx, encoded, cbOut, reinterpret_cast<const uint8_t*>(fResult));
         assert(dstLen < sizeof(encoded));
         dstLen += base64_encode_final(&ctx, encoded+dstLen);

diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc
index 5f23e52395ce8f099682a1b1b348116997121cb1..3ad64b8f40e2aef9b8b1dfecf016289a59fd69c5 100644 (file)
--- a/src/adaptation/icap/ModXact.cc
+++ b/src/adaptation/icap/ModXact.cc
@@ -1399,12 +1399,18 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf)
         String vh=virgin.header->header.getById(Http::HdrType::PROXY_AUTHORIZATION);
         buf.appendf("Proxy-Authorization: " SQUIDSTRINGPH "\r\n", SQUIDSTRINGPRINT(vh));
     } else if (request->extacl_user.size() > 0 && request->extacl_passwd.size() > 0) {
+        const auto userLen = request->extacl_user.size();
+        const auto passwdLen = request->extacl_passwd.size();
+        // +1 for the ':' separator between user and passwd
+        const auto plainLen = userLen + 1 + passwdLen;
+        if (plainLen > MAX_LOGIN_SZ)
+            throw TextException("extacl credentials too long for Proxy-Authorization", Here());
+        char base64buf[base64_encode_len(MAX_LOGIN_SZ)];
         struct base64_encode_ctx ctx;
         base64_encode_init(&ctx);
-        char base64buf[base64_encode_len(MAX_LOGIN_SZ)];
-        size_t resultLen = base64_encode_update(&ctx, base64buf, request->extacl_user.size(), reinterpret_cast<const uint8_t*>(request->extacl_user.rawBuf()));
+        auto resultLen = base64_encode_update(&ctx, base64buf, userLen, reinterpret_cast<const uint8_t*>(request->extacl_user.rawBuf()));
         resultLen += base64_encode_update(&ctx, base64buf+resultLen, 1, reinterpret_cast<const uint8_t*>(":"));
-        resultLen += base64_encode_update(&ctx, base64buf+resultLen, request->extacl_passwd.size(), reinterpret_cast<const uint8_t*>(request->extacl_passwd.rawBuf()));
+        resultLen += base64_encode_update(&ctx, base64buf+resultLen, passwdLen, reinterpret_cast<const uint8_t*>(request->extacl_passwd.rawBuf()));
         resultLen += base64_encode_final(&ctx, base64buf+resultLen);
         buf.appendf("Proxy-Authorization: Basic %.*s\r\n", (int)resultLen, base64buf);
     }

diff --git a/src/http.cc b/src/http.cc
index ef4ecf076d46772c00c1a8298c96dbc6a72e7b7e..acb2299f2cfa02ff76509aa09e2b01c896940920 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1850,8 +1850,12 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe
             username = request->auth_user_request->username();
 #endif
 
-        blen = base64_encode_update(&ctx, loginbuf, strlen(username), reinterpret_cast<const uint8_t*>(username));
-        blen += base64_encode_update(&ctx, loginbuf+blen, strlen(request->peer_login +1), reinterpret_cast<const uint8_t*>(request->peer_login +1));
+        const auto usernameLen = strlen(username);
+        const auto suffixLen = strlen(request->peer_login + 1);
+        if (usernameLen + suffixLen > MAX_LOGIN_SZ)
+            throw TextException("peer login credentials too long", Here());
+        blen = base64_encode_update(&ctx, loginbuf, usernameLen, reinterpret_cast<const uint8_t*>(username));
+        blen += base64_encode_update(&ctx, loginbuf+blen, suffixLen, reinterpret_cast<const uint8_t*>(request->peer_login +1));
         blen += base64_encode_final(&ctx, loginbuf+blen);
         httpHeaderPutStrf(hdr_out, header, "Basic %.*s", (int)blen, loginbuf);
         return;
@@ -1862,9 +1866,14 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe
             (strcmp(request->peer_login, "PASS") == 0 ||
              strcmp(request->peer_login, "PROXYPASS") == 0)) {
 
-        blen = base64_encode_update(&ctx, loginbuf, request->extacl_user.size(), reinterpret_cast<const uint8_t*>(request->extacl_user.rawBuf()));
+        const auto userLen = request->extacl_user.size();
+        const auto passwdLen = request->extacl_passwd.size();
+        // +1 for the ':' separator between user and passwd
+        if (userLen + 1 + passwdLen > MAX_LOGIN_SZ)
+            throw TextException("extacl credentials too long for peer login", Here());
+        blen = base64_encode_update(&ctx, loginbuf, userLen, reinterpret_cast<const uint8_t*>(request->extacl_user.rawBuf()));
         blen += base64_encode_update(&ctx, loginbuf+blen, 1, reinterpret_cast<const uint8_t*>(":"));
-        blen += base64_encode_update(&ctx, loginbuf+blen, request->extacl_passwd.size(), reinterpret_cast<const uint8_t*>(request->extacl_passwd.rawBuf()));
+        blen += base64_encode_update(&ctx, loginbuf+blen, passwdLen, reinterpret_cast<const uint8_t*>(request->extacl_passwd.rawBuf()));
         blen += base64_encode_final(&ctx, loginbuf+blen);
         httpHeaderPutStrf(hdr_out, header, "Basic %.*s", (int)blen, loginbuf);
         return;
@@ -1894,7 +1903,10 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe
     }
 #endif /* HAVE_KRB5 && HAVE_GSSAPI */
 
-    blen = base64_encode_update(&ctx, loginbuf, strlen(request->peer_login), reinterpret_cast<const uint8_t*>(request->peer_login));
+    const auto loginLen = strlen(request->peer_login);
+    if (loginLen > MAX_LOGIN_SZ)
+        throw TextException("peer_login too long", Here());
+    blen = base64_encode_update(&ctx, loginbuf, loginLen, reinterpret_cast<const uint8_t*>(request->peer_login));
     blen += base64_encode_final(&ctx, loginbuf+blen);
     httpHeaderPutStrf(hdr_out, header, "Basic %.*s", (int)blen, loginbuf);
     return;
@@ -2018,6 +2030,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
     /* append Authorization if known in URL, not in header and going direct */
     if (!hdr_out->has(Http::HdrType::AUTHORIZATION)) {
         if (flags.toOrigin && !request->url.userInfo().isEmpty()) {
+            Assure(request->url.userInfo().length() < MAX_URL*2);
             static char result[base64_encode_len(MAX_URL*2)]; // should be big enough for a single URI segment
             struct base64_encode_ctx ctx;
             base64_encode_init(&ctx);

diff --git a/src/peer_proxy_negotiate_auth.cc b/src/peer_proxy_negotiate_auth.cc
index eb937e5ec3888ca3082b0f370f0d9d5633a1d60f..c0a44f709cfd503fcf222d4d9607d0d9fdba353c 100644 (file)
--- a/src/peer_proxy_negotiate_auth.cc
+++ b/src/peer_proxy_negotiate_auth.cc
@@ -13,6 +13,7 @@
 #include "squid.h"
 
 #if HAVE_AUTH_MODULE_NEGOTIATE && HAVE_KRB5 && HAVE_GSSAPI
+#include "base/Assure.h"
 #include "base64.h"
 #include "compat/krb5.h"
 #include "debug/Stream.h"
@@ -546,6 +547,7 @@ char *peer_proxy_negotiate_auth(char *principal_name, const char * const proxy,
         static char b64buf[8192]; // XXX: 8KB only because base64_encode_bin() used to.
         struct base64_encode_ctx ctx;
         base64_encode_init(&ctx);
+        Assure(base64_encode_len(output_token.length) < sizeof(b64buf));
         size_t blen = base64_encode_update(&ctx, b64buf, output_token.length, reinterpret_cast<const uint8_t*>(output_token.value));
         blen += base64_encode_final(&ctx, b64buf+blen);
         b64buf[blen] = '\0';