From 2b7e1b820bb8429ee840c3a0558ce5fbdf719427 Mon Sep 17 00:00:00 2001 From: Alhuda Khan Date: Tue, 20 Oct 2026 14:05:20 +0530 Subject: [PATCH] imapauth: quote LOGIN arguments and refuse passwords with line breaks --- modules/imapauth.cpp | 16 ++++++++++++++-- test/integration/tests/modules.cpp | 24 +++++++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/modules/imapauth.cpp b/modules/imapauth.cpp index 042b1dfb..56c14edd 100644 --- a/modules/imapauth.cpp +++ b/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() + - "\r\n"); + Write("AUTH LOGIN " + Quote(sUsername) + " " + + Quote(m_spAuth->GetPassword()) + "\r\n"); } else if (sLine.Left(5) == "AUTH ") { CUser* pUser = CZNC::Get().FindUser(m_spAuth->GetUsername()); diff --git a/test/integration/tests/modules.cpp b/test/integration/tests/modules.cpp index fe315901..54732252 100644 --- a/test/integration/tests/modules.cpp +++ b/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";