From 026b88e2fa4e7d70732507b1ae9b87eaa1a29873 Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Sat, 25 Apr 2026 10:34:06 +0800 Subject: [PATCH 1/3] ZNCString: avoid left shift of negative value in Base64Decode base64_table uses the sentinel 0xff for bytes outside the base64 alphabet. The old code read that through (char), producing signed -1, which made the three (c << N) expressions in Base64Decode undefined behaviour when the input contained any invalid byte. Keep c and c1 as unsigned char so the shifts are well-defined. Reachable pre-auth via CHTTPSock::ReadLine for the Authorization: Basic value. --- src/ZNCString.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/ZNCString.cpp b/src/ZNCString.cpp index abda9e83..cf9c28ca 100644 --- a/src/ZNCString.cpp +++ b/src/ZNCString.cpp @@ -1087,21 +1087,25 @@ unsigned long CString::Base64Decode(CString& sRet) const { sTmp.Replace("\n", ""); const char* in = sTmp.c_str(); - char c, c1, *p; + // Keep c and c1 unsigned so left-shifts are well-defined even when the + // input contains bytes outside the base64 alphabet (base64_table maps + // those to the sentinel 0xff, which used to become signed -1). + unsigned char c, c1; + char* p; unsigned long i; unsigned long uLen = sTmp.size(); char* out = new char[uLen + 1]{}; for (i = 0, p = out; i < uLen; i++) { - c = (char)base64_table[(unsigned char)in[i++]]; - c1 = (char)base64_table[(unsigned char)in[i++]]; + c = base64_table[(unsigned char)in[i++]]; + c1 = base64_table[(unsigned char)in[i++]]; *p++ = char((c << 2) | ((c1 >> 4) & 0x3)); if (i < uLen) { if (in[i] == '=') { break; } - c = (char)base64_table[(unsigned char)in[i]]; + c = base64_table[(unsigned char)in[i]]; *p++ = char(((c1 << 4) & 0xf0) | ((c >> 2) & 0xf)); } @@ -1110,7 +1114,7 @@ unsigned long CString::Base64Decode(CString& sRet) const { break; } *p++ = char(((c << 6) & 0xc0) | - (char)base64_table[(unsigned char)in[i]]); + base64_table[(unsigned char)in[i]]); } } From d60f489c27c6d89e44e37cc276876d615a61586c Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Sat, 25 Apr 2026 17:34:11 +0800 Subject: [PATCH 2/3] test: cover Base64Decode round-trip and out-of-alphabet bytes (#2013) --- test/StringTest.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/StringTest.cpp b/test/StringTest.cpp index 160e695d..d1127454 100644 --- a/test/StringTest.cpp +++ b/test/StringTest.cpp @@ -179,6 +179,35 @@ TEST(StringTest, NamedFormat) { EXPECT_EQ(CString::NamedFormat(CS("\\{x{a}y{a}"), m), "{xbyb"); } +TEST(StringTest, Base64) { + // Round-trip regression: encode/decode of normal text still works. + CString sIn = "Hello, World!"; + CString sEncoded = sIn.Base64Encode_n(); + EXPECT_EQ(sEncoded, "SGVsbG8sIFdvcmxkIQ=="); + EXPECT_EQ(sEncoded.Base64Decode_n(), sIn); + + // All-zero bytes round-trip cleanly. + CString sBin = CString("\0\0\0", 3); + EXPECT_EQ(sBin.Base64Encode_n().Base64Decode_n(), sBin); + + // Inputs containing bytes outside the base64 alphabet must not invoke + // undefined behaviour. base64_table maps such bytes to the sentinel + // 0xff; the old code cast that to signed char (-1) and then evaluated + // (c << 2) and (c << 6), both UB on signed shifts. Run under UBSan to + // catch a regression of #2013. + CString sInvalid; + sInvalid += '\xff'; + sInvalid += '\xff'; + sInvalid += '\xff'; + sInvalid += '\xff'; + CString sOut; + sInvalid.Base64Decode(sOut); // must not crash or trigger UB + + // Mixed-validity input (a single non-alphabet byte inside a quad). + CString sMixed = CString("AA\xffA", 4); + sMixed.Base64Decode(sOut); // must not crash or trigger UB +} + TEST(StringTest, Hash) { EXPECT_EQ(CS("").MD5(), "d41d8cd98f00b204e9800998ecf8427e"); EXPECT_EQ(CS("a").MD5(), "0cc175b9c0f1b6a831c399e269772661"); From 099895b1f06fa69940f9cea30ba36e4f2a9c526b Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Wed, 29 Apr 2026 19:33:30 +0800 Subject: [PATCH 3/3] test: fix \xff hex escape parsing in Base64 test for stricter compilers GCC parses "AA\xffA" greedily as \xffA (next character is a hex digit), which is out of range for char and breaks the Linux CI build. Split the literal into "AA\xff" "A" so the escape resolves before the next string, yielding the intended four bytes (A, A, 0xff, A). --- test/StringTest.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/StringTest.cpp b/test/StringTest.cpp index d1127454..4bde1b4c 100644 --- a/test/StringTest.cpp +++ b/test/StringTest.cpp @@ -204,7 +204,9 @@ TEST(StringTest, Base64) { sInvalid.Base64Decode(sOut); // must not crash or trigger UB // Mixed-validity input (a single non-alphabet byte inside a quad). - CString sMixed = CString("AA\xffA", 4); + // Split the literal so GCC does not parse \xff and the following A as + // a single \xffA hex escape (out of range for char). + CString sMixed = CString("AA\xff" "A", 4); sMixed.Base64Decode(sOut); // must not crash or trigger UB }