FTP: Ignore credentials with a NUL-prefixed username (#1557)

    FATAL: FTP login parsing destroyed username info

This bug was discovered and detailed by Joshua Rogers at
https://megamansec.github.io/Squid-Security-Audit/ftp-fatal.html
where it was filed as "FTP Authentication Crash".

diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc
index 783006a715bd99e80adf6f8a77830ea747ad12dc..c072a3c1b46721a9ff3adc1110e9fe5e2be53eb0 100644 (file)
--- a/src/clients/FtpGateway.cc
+++ b/src/clients/FtpGateway.cc
@@ -404,6 +404,12 @@ Ftp::Gateway::loginParser(const SBuf &login, bool escaped)
     if (login.isEmpty())
         return;
 
+    if (!login[0]) {
+        debugs(9, 2, "WARNING: Ignoring FTP credentials that start with a NUL character");
+        // TODO: Either support credentials with NUL characters (in any position) or ban all of them.
+        return;
+    }
+
     const SBuf::size_type colonPos = login.find(':');
 
     /* If there was a username part with at least one character use it.
@@ -1042,9 +1048,8 @@ Ftp::Gateway::checkAuth(const HttpHeader * req_hdr)
     /* Test URL login syntax. Overrides any headers received. */
     loginParser(request->url.userInfo(), true);
 
-    /* name is missing. thats fatal. */
-    if (!user[0])
-        fatal("FTP login parsing destroyed username info");
+    // XXX: We we keep default "anonymous" instead of properly supporting empty usernames.
+    Must(user[0]);
 
     /* name + password == success */
     if (password[0])