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 e04f3624f01d8835ce4aa9c806ac31f8000e89bc..7fb43df3ea9b4b8b26627690c38c94e8ed6f3edb 100644 (file)
--- a/src/clients/FtpGateway.cc
+++ b/src/clients/FtpGateway.cc
@@ -402,6 +402,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. that's fatal. */
-    if (!user[0])
-        fatal("FTP login parsing destroyed username info");
+    // XXX: We we keep default "anonymous" instead of properly supporting empty usernames.
+    Assure(user[0]);
 
     /* name + password == success */
     if (password[0])