Improve base64Decode bounds checking
Index the decode array with an unsigned char rather than a signed int
(which could accees outside the bounds of decodingData, leading to
undefined behavior).
Add unit tests for basic decoding functionality.
Remove duplicate unused base64 functions.
Tested: ran webtest and observed that previously failing
Base64DecodeNonAscii now passes. Also tested basic auth:
$ curl -vku root:0penBmc https://<ip>/redfish/v1/Managers/bmc
...
< HTTP/1.1 200 OK
...
Change-Id: I9f9e32650b1796f9fc0b2b25d482dffa35fac72d
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
diff --git a/http/ut/utility_test.cpp b/http/ut/utility_test.cpp
new file mode 100644
index 0000000..0f40756
--- /dev/null
+++ b/http/ut/utility_test.cpp
@@ -0,0 +1,18 @@
+#include "utility.h"
+
+#include "gmock/gmock.h"
+
+TEST(Utility, Base64DecodeAuthString)
+{
+ std::string authString("dXNlcm40bWU6cGFzc3cwcmQ=");
+ std::string result;
+ EXPECT_TRUE(crow::utility::base64Decode(authString, result));
+ EXPECT_EQ(result, "usern4me:passw0rd");
+}
+
+TEST(Utility, Base64DecodeNonAscii)
+{
+ std::string junkString("\xff\xee\xdd\xcc\x01\x11\x22\x33");
+ std::string result;
+ EXPECT_FALSE(crow::utility::base64Decode(junkString, result));
+}
diff --git a/http/utility.h b/http/utility.h
index fad212f..20e85b4 100644
--- a/http/utility.h
+++ b/http/utility.h
@@ -633,7 +633,7 @@
{
static const char nop = static_cast<char>(-1);
// See note on encoding_data[] in above function
- static const char decodingData[] = {
+ static const std::array<char, 256> decodingData = {
nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop,
nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop,
nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop, nop,
@@ -660,6 +660,14 @@
output.clear();
output.reserve(((inputLength + 2) / 3) * 4);
+ auto getCodeValue = [](char c) {
+ auto code = static_cast<unsigned char>(c);
+ // Ensure we cannot index outside the bounds of the decoding array
+ static_assert(std::numeric_limits<decltype(code)>::max() <
+ decodingData.size());
+ return decodingData[code];
+ };
+
// for each 4-bytes sequence from the input, extract 4 6-bits sequences by
// dropping first two bits
// and regenerate into 3 8-bits sequences
@@ -671,7 +679,7 @@
char base64code2 = 0; // initialized to 0 to suppress warnings
char base64code3;
- base64code0 = decodingData[static_cast<int>(input[i])]; // NOLINT
+ base64code0 = getCodeValue(input[i]);
if (base64code0 == nop)
{ // non base64 character
return false;
@@ -681,7 +689,7 @@
// byte output
return false;
}
- base64code1 = decodingData[static_cast<int>(input[i])]; // NOLINT
+ base64code1 = getCodeValue(input[i]);
if (base64code1 == nop)
{ // non base64 character
return false;
@@ -696,7 +704,7 @@
{ // padding , end of input
return (base64code1 & 0x0f) == 0;
}
- base64code2 = decodingData[static_cast<int>(input[i])]; // NOLINT
+ base64code2 = getCodeValue(input[i]);
if (base64code2 == nop)
{ // non base64 character
return false;
@@ -712,7 +720,7 @@
{ // padding , end of input
return (base64code2 & 0x03) == 0;
}
- base64code3 = decodingData[static_cast<int>(input[i])]; // NOLINT
+ base64code3 = getCodeValue(input[i]);
if (base64code3 == nop)
{ // non base64 character
return false;