From: Amos Jeffries <squid3@treenet.co.nz>
Date: Fri, 3 Mar 2017 11:41:07 +0000 (+1300)
Subject: Bug 4671 pt3: remove limit on FTP realm strings
X-Git-Tag: M-staged-PR71~232
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3fdf6904e3d7e87e6e5a47c893352670d7286a69;p=thirdparty%2Fsquid.git

Bug 4671 pt3: remove limit on FTP realm strings

Convert ftpRealm() from generating char* to SBuf. This fixes issues identified
by GCC 7 where the realm string may be longer than the available buffer and
gets truncated.
The size of the buffer was making the occurance rather rare, but it is still
possible.
---

diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc
index da0d5d2a09..aa4d97aa87 100644
--- a/src/clients/FtpGateway.cc
+++ b/src/clients/FtpGateway.cc
@@ -153,8 +153,8 @@ public:
     virtual void timeout(const CommTimeoutCbParams &io);
     void ftpAcceptDataConnection(const CommAcceptCbParams &io);
 
-    static HttpReply *ftpAuthRequired(HttpRequest * request, const char *realm);
-    const char *ftpRealm(void);
+    static HttpReply *ftpAuthRequired(HttpRequest * request, SBuf &realm);
+    SBuf ftpRealm();
     void loginFailed(void);
 
     virtual void haveParsedReplyHeaders();
@@ -1162,7 +1162,8 @@ Ftp::Gateway::start()
 {
     if (!checkAuth(&request->header)) {
         /* create appropriate reply */
-        HttpReply *reply = ftpAuthRequired(request, ftpRealm());
+        SBuf realm(ftpRealm()); // local copy so SBuf wont disappear too early
+        HttpReply *reply = ftpAuthRequired(request, realm);
         entry->replaceHttpReply(reply);
         serverComplete();
         return;
@@ -1263,7 +1264,9 @@ Ftp::Gateway::loginFailed()
 
 #if HAVE_AUTH_MODULE_BASIC
     /* add Authenticate header */
-    newrep->header.putAuth("Basic", ftpRealm());
+    // XXX: performance regression. c_str() may reallocate
+    SBuf realm(ftpRealm()); // local copy so SBuf wont disappear too early
+    newrep->header.putAuth("Basic", realm.c_str());
 #endif
 
     // add it to the store entry for response....
@@ -1271,18 +1274,19 @@ Ftp::Gateway::loginFailed()
     serverComplete();
 }
 
-const char *
+SBuf
 Ftp::Gateway::ftpRealm()
 {
-    static char realm[8192];
+    SBuf realm;
 
     /* This request is not fully authenticated */
-    if (!request) {
-        snprintf(realm, 8192, "FTP %s unknown", user);
-    } else if (request->url.port() == 21) {
-        snprintf(realm, 8192, "FTP %s %s", user, request->url.host());
-    } else {
-        snprintf(realm, 8192, "FTP %s %s port %d", user, request->url.host(), request->url.port());
+    realm.appendf("FTP %s ", user);
+    if (!request)
+        realm.append("unknown", 7);
+    else {
+        realm.append(request->url.host());
+        if (request->url.port() != 21)
+            realm.appendf(" port %d", request->url.port());
     }
     return realm;
 }
@@ -2642,13 +2646,14 @@ Ftp::Gateway::haveParsedReplyHeaders()
 }
 
 HttpReply *
-Ftp::Gateway::ftpAuthRequired(HttpRequest * request, const char *realm)
+Ftp::Gateway::ftpAuthRequired(HttpRequest * request, SBuf &realm)
 {
     ErrorState err(ERR_CACHE_ACCESS_DENIED, Http::scUnauthorized, request);
     HttpReply *newrep = err.BuildHttpReply();
 #if HAVE_AUTH_MODULE_BASIC
     /* add Authenticate header */
-    newrep->header.putAuth("Basic", realm);
+    // XXX: performance regression. c_str() may reallocate
+    newrep->header.putAuth("Basic", realm.c_str());
 #endif
     return newrep;
 }