Reject incorrect base64 in HTTP auth

sipa · MacroFake · commit a4377a084363 · 2022-04-27T14:12:55.000+02:00
In addition, to make sure that no call site ignores the invalid
decoding status, make the pf_invalid argument mandatory.
diff --git a/src/httprpc.cpp b/src/httprpc.cpp
@@ -132,7 +132,9 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna
     if (strAuth.substr(0, 6) != "Basic ")
         return false;
     std::string strUserPass64 = TrimString(strAuth.substr(6));
-    std::string strUserPass = DecodeBase64(strUserPass64);
+    bool invalid;
+    std::string strUserPass = DecodeBase64(strUserPass64, &invalid);
+    if (invalid) return false;
 
     if (strUserPass.find(':') != std::string::npos)
         strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':'));
diff --git a/src/test/base32_tests.cpp b/src/test/base32_tests.cpp
@@ -22,7 +22,9 @@ BOOST_AUTO_TEST_CASE(base32_testvectors)
         BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);
         strEnc = EncodeBase32(vstrIn[i], false);
         BOOST_CHECK_EQUAL(strEnc, vstrOutNoPadding[i]);
-        std::string strDec = DecodeBase32(vstrOut[i]);
+        bool invalid;
+        std::string strDec = DecodeBase32(vstrOut[i], &invalid);
+        BOOST_CHECK(!invalid);
         BOOST_CHECK_EQUAL(strDec, vstrIn[i]);
     }
 
diff --git a/src/test/base64_tests.cpp b/src/test/base64_tests.cpp
@@ -19,7 +19,9 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
     {
         std::string strEnc = EncodeBase64(vstrIn[i]);
         BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);
-        std::string strDec = DecodeBase64(strEnc);
+        bool invalid;
+        std::string strDec = DecodeBase64(strEnc, &invalid);
+        BOOST_CHECK(!invalid);
         BOOST_CHECK_EQUAL(strDec, vstrIn[i]);
     }
 
diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
@@ -167,17 +167,15 @@ std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid)
         ++p;
     }
     valid = valid && (p - e) % 4 == 0 && p - q < 4;
-    if (pf_invalid) *pf_invalid = !valid;
+    *pf_invalid = !valid;
 
     return ret;
 }
 
 std::string DecodeBase64(const std::string& str, bool* pf_invalid)
 {
     if (!ValidAsCString(str)) {
-        if (pf_invalid) {
-            *pf_invalid = true;
-        }
+        *pf_invalid = true;
         return {};
     }
     std::vector<unsigned char> vchRet = DecodeBase64(str.c_str(), pf_invalid);
@@ -245,17 +243,15 @@ std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid)
         ++p;
     }
     valid = valid && (p - e) % 8 == 0 && p - q < 8;
-    if (pf_invalid) *pf_invalid = !valid;
+    *pf_invalid = !valid;
 
     return ret;
 }
 
 std::string DecodeBase32(const std::string& str, bool* pf_invalid)
 {
     if (!ValidAsCString(str)) {
-        if (pf_invalid) {
-            *pf_invalid = true;
-        }
+        *pf_invalid = true;
         return {};
     }
     std::vector<unsigned char> vchRet = DecodeBase32(str.c_str(), pf_invalid);
diff --git a/src/util/strencodings.h b/src/util/strencodings.h
@@ -64,13 +64,13 @@ bool IsHex(std::string_view str);
 * Return true if the string is a hex number, optionally prefixed with "0x"
 */
 bool IsHexNumber(std::string_view str);
-std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid = nullptr);
-std::string DecodeBase64(const std::string& str, bool* pf_invalid = nullptr);
+std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid);
+std::string DecodeBase64(const std::string& str, bool* pf_invalid);
 std::string EncodeBase64(Span<const unsigned char> input);
 inline std::string EncodeBase64(Span<const std::byte> input) { return EncodeBase64(MakeUCharSpan(input)); }
 inline std::string EncodeBase64(const std::string& str) { return EncodeBase64(MakeUCharSpan(str)); }
-std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid = nullptr);
-std::string DecodeBase32(const std::string& str, bool* pf_invalid = nullptr);
+std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid);
+std::string DecodeBase32(const std::string& str, bool* pf_invalid);
 
 /**
  * Base32 encode.

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,9 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)`
`19`	`19`	`{`
`20`	`20`	`std::string strEnc = EncodeBase64(vstrIn[i]);`
`21`	`21`	`BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);`
`22`		`- std::string strDec = DecodeBase64(strEnc);`
	`22`	`+ bool invalid;`
	`23`	`+ std::string strDec = DecodeBase64(strEnc, &invalid);`
	`24`	`+ BOOST_CHECK(!invalid);`
`23`	`25`	`BOOST_CHECK_EQUAL(strDec, vstrIn[i]);`
`24`	`26`	`}`
`25`	`27`
Original file line number	Diff line number	Diff line change
`@@ -167,17 +167,15 @@ std::vector<unsigned char> DecodeBase64(const char* p, bool* pf_invalid)`
`167`	`167`	`++p;`
`168`	`168`	`}`
`169`	`169`	`valid = valid && (p - e) % 4 == 0 && p - q < 4;`
`170`		`- if (pf_invalid) *pf_invalid = !valid;`
	`170`	`+ *pf_invalid = !valid;`
`171`	`171`
`172`	`172`	`return ret;`
`173`	`173`	`}`
`174`	`174`
`175`	`175`	`std::string DecodeBase64(const std::string& str, bool* pf_invalid)`
`176`	`176`	`{`
`177`	`177`	`if (!ValidAsCString(str)) {`
`178`		`- if (pf_invalid) {`
`179`		`- *pf_invalid = true;`
`180`		`- }`
	`178`	`+ *pf_invalid = true;`
`181`	`179`	`return {};`
`182`	`180`	`}`
`183`	`181`	`std::vector<unsigned char> vchRet = DecodeBase64(str.c_str(), pf_invalid);`
`@@ -245,17 +243,15 @@ std::vector<unsigned char> DecodeBase32(const char* p, bool* pf_invalid)`
`245`	`243`	`++p;`
`246`	`244`	`}`
`247`	`245`	`valid = valid && (p - e) % 8 == 0 && p - q < 8;`
`248`		`- if (pf_invalid) *pf_invalid = !valid;`
	`246`	`+ *pf_invalid = !valid;`
`249`	`247`
`250`	`248`	`return ret;`
`251`	`249`	`}`
`252`	`250`
`253`	`251`	`std::string DecodeBase32(const std::string& str, bool* pf_invalid)`
`254`	`252`	`{`
`255`	`253`	`if (!ValidAsCString(str)) {`
`256`		`- if (pf_invalid) {`
`257`		`- *pf_invalid = true;`
`258`		`- }`
	`254`	`+ *pf_invalid = true;`
`259`	`255`	`return {};`
`260`	`256`	`}`
`261`	`257`	`std::vector<unsigned char> vchRet = DecodeBase32(str.c_str(), pf_invalid);`