Merge pull request #2013 from MarkLee131/fix/base64-decode-ub

ZNCString: avoid left shift of negative value in Base64Decode

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]]);
         }
     }
 

test/StringTest.cpp

@@ -179,6 +179,37 @@ 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).
+    // 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
+}
+
 TEST(StringTest, Hash) {
     EXPECT_EQ(CS("").MD5(), "d41d8cd98f00b204e9800998ecf8427e");
     EXPECT_EQ(CS("a").MD5(), "0cc175b9c0f1b6a831c399e269772661");