Merge pull request #2057 from kitkatt7/imapauth-quote-login

imapauth: quote LOGIN arguments and refuse passwords with line breaks

modules/imapauth.cpp

@@ -40,6 +40,10 @@ class CIMAPSock : public CSocket {
     void ReadLine(const CString& sLine) override;
 
   private:
+    static CString Quote(const CString& s) {
+        return "\"" + s.Replace_n("\\", "\\\\").Replace_n("\"", "\\\"") + "\"";
+    }
+
   protected:
     CIMAPAuthMod* m_pIMAPMod;
     bool m_bSentLogin;
@@ -93,6 +97,14 @@ class CIMAPAuthMod : public CModule {
             return HALT;
         }
 
+        // CR, LF and NUL can't appear in an IMAP quoted string, and a line
+        // break would let the password inject further IMAP commands.
+        const CString sForbidden("\r\n\0", 3);
+        if (Auth->GetPassword().find_first_of(sForbidden) != CString::npos) {
+            Auth->RefuseLogin("Invalid Password");
+            return HALT;
+        }
+
         if (pUser &&
             m_Cache.HasItem(CString(Auth->GetUsername() + ":" +
                                     Auth->GetPassword()).MD5())) {
@@ -140,8 +152,8 @@ void CIMAPSock::ReadLine(const CString& sLine) {
             }
         }
 
-        Write("AUTH LOGIN " + sUsername + " " + m_spAuth->GetPassword() +
+        Write("AUTH LOGIN " + Quote(sUsername) + " " +
-              "\r\n");
+              Quote(m_spAuth->GetPassword()) + "\r\n");
     } else if (sLine.Left(5) == "AUTH ") {
         CUser* pUser = CZNC::Get().FindUser(m_spAuth->GetUsername());
 

test/integration/tests/modules.cpp

@@ -561,12 +561,34 @@ TEST_F(ZNCTest, SaslAuthPlainImapAuth) {
     ASSERT_TRUE(imap.waitForNewConnection(30000 /* msec */));
     auto imapsock = WrapIO(imap.nextPendingConnection());
     imapsock.Write("* OK IMAP4rev1 Service Ready");
-    imapsock.ReadUntil("AUTH LOGIN user@mail.test.com hunter3");
+    imapsock.ReadUntil("AUTH LOGIN \"user@mail.test.com\" \"hunter3\"");
     imapsock.Write("AUTH OK");
 
     client2.ReadUntil(":irc.znc.in 903 foo :SASL authentication successful");
 }
 
+TEST_F(ZNCTest, SaslAuthPlainImapAuthLineBreak) {
+    auto znc = Run();
+    auto ircd = ConnectIRCd();
+    QTcpServer imap;
+    ASSERT_TRUE(imap.listen(QHostAddress::LocalHost)) << imap.errorString().toStdString();
+    auto client = LoginClient();
+    client.Write(QStringLiteral("znc loadmod imapauth 127.0.0.1 %1")
+                     .arg(imap.serverPort())
+                     .toUtf8());
+    client.ReadUntil("Loaded");
+
+    auto client2 = ConnectClient();
+    client2.Write("NICK foo");
+    client2.Write("CAP REQ :sasl");
+    client2.Write("USER bar");
+    client2.Write("AUTHENTICATE PLAIN");
+    client2.Write("AUTHENTICATE " +
+                  QByteArrayLiteral("\0user\0x\r\nAUTH2 LOGIN a b").toBase64());
+    client2.ReadUntil(":irc.znc.in 904 foo :Invalid Password");
+    EXPECT_FALSE(imap.hasPendingConnections());
+}
+
 TEST_F(ZNCTest, SaslAuthExternal) {
 #ifndef HAVE_LIBSSL
     GTEST_SKIP() << "SSL is disabled";