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/CMakeLists.txt b/CMakeLists.txt
index 9d5aa63..6181fef 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -381,6 +381,7 @@
set (UT_FILES src/gtest_main.cpp src/msan_test.cpp
redfish-core/ut/privileges_test.cpp
redfish-core/ut/lock_test.cpp
+ http/ut/utility_test.cpp
${CMAKE_BINARY_DIR}/include/bmcweb/blns.hpp) # big list of naughty
# strings
add_custom_command (OUTPUT ${CMAKE_BINARY_DIR}/include/bmcweb/blns.hpp
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;
diff --git a/src/base64.cpp b/src/base64.cpp
deleted file mode 100644
index e32090a..0000000
--- a/src/base64.cpp
+++ /dev/null
@@ -1,167 +0,0 @@
-#include <base64.hpp>
-
-namespace base64
-{
-bool base64_encode(const std::string& input, std::string& output)
-{
- // As is, this array is 64 bytes long, which should be greater than the max
- // of 0b00111111 when indexed NOLINT calls below are to silence clang-tidy
- // warnings about this
- constexpr std::array<char, 64> encoding_data = {
- 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M',
- 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z',
- 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
- 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
- '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '+', '/'};
-
- size_t input_length = input.size();
-
- // allocate space for output string
- output.clear();
- output.reserve(((input_length + 2) / 3) * 4);
-
- // for each 3-bytes sequence from the input, extract 4 6-bits sequences and
- // encode using
- // encoding_data lookup table.
- // if input do not contains enough chars to complete 3-byte sequence,use pad
- // char '='
- for (size_t i = 0; i < input_length; i++)
- {
- int base64code0 = 0;
- int base64code1 = 0;
- int base64code2 = 0;
- int base64code3 = 0;
-
- base64code0 = (input[i] >> 2) & 0x3f; // 1-byte 6 bits
-
- output += encoding_data[base64code0]; // NOLINT
- base64code1 = (input[i] << 4) & 0x3f; // 1-byte 2 bits +
-
- if (++i < input_length)
- {
- base64code1 |= (input[i] >> 4) & 0x0f; // 2-byte 4 bits
- output += encoding_data[base64code1]; // NOLINT
- base64code2 = (input[i] << 2) & 0x3f; // 2-byte 4 bits +
-
- if (++i < input_length)
- {
- base64code2 |= (input[i] >> 6) & 0x03; // 3-byte 2 bits
- base64code3 = input[i] & 0x3f; // 3-byte 6 bits
- output += encoding_data[base64code2]; // NOLINT
- output += encoding_data[base64code3]; // NOLINT
- }
- else
- {
- output += encoding_data[base64code2]; // NOLINT
- output += '=';
- }
- }
- else
- {
- output += encoding_data[base64code1]; // NOLINT
- output += '=';
- output += '=';
- }
- }
-
- return true;
-}
-
-bool base64_decode(const std::string& input, std::string& output)
-{
- constexpr char nop = -1;
- // See note on encoding_data[] in above function
- constexpr std::array<char, 256> decoding_data = {
- 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,
- nop, 62, nop, nop, nop, 63, 52, 53, 54, 55, 56, 57, 58, 59,
- 60, 61, nop, nop, nop, nop, nop, nop, nop, 0, 1, 2, 3, 4,
- 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18,
- 19, 20, 21, 22, 23, 24, 25, nop, nop, nop, nop, nop, nop, 26,
- 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
- 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 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, 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, 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, nop, nop, nop,
- nop, nop, nop, nop};
-
- size_t input_length = input.size();
-
- // allocate space for output string
- output.clear();
- output.reserve(((input_length + 2) / 3) * 4);
-
- // 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
-
- for (size_t i = 0; i < input_length; i++)
- {
- char base64code0;
- char base64code1;
- char base64code2 = 0; // initialized to 0 to suppress warnings
- char base64code3;
-
- base64code0 = decoding_data[static_cast<size_t>(input[i])]; // NOLINT
- if (base64code0 == nop)
- { // non base64 character
- return false;
- }
- if (!(++i < input_length))
- { // we need at least two input bytes for first
- // byte output
- return false;
- }
- base64code1 = decoding_data[static_cast<size_t>(input[i])]; // NOLINT
- if (base64code1 == nop)
- { // non base64 character
- return false;
- }
- output +=
- static_cast<char>((base64code0 << 2) | ((base64code1 >> 4) & 0x3));
-
- if (++i < input_length)
- {
- char c = input[i];
- if (c == '=')
- { // padding , end of input
- return (base64code1 & 0x0f) == 0;
- }
- base64code2 =
- decoding_data[static_cast<size_t>(input[i])]; // NOLINT
- if (base64code2 == nop)
- { // non base64 character
- return false;
- }
- output += static_cast<char>(((base64code1 << 4) & 0xf0) |
- ((base64code2 >> 2) & 0x0f));
- }
-
- if (++i < input_length)
- {
- char c = input[i];
- if (c == '=')
- { // padding , end of input
- return (base64code2 & 0x03) == 0;
- }
- base64code3 =
- decoding_data[static_cast<size_t>(input[i])]; // NOLINT
- if (base64code3 == nop)
- { // non base64 character
- return false;
- }
- output +=
- static_cast<char>((((base64code2 << 6) & 0xc0) | base64code3));
- }
- }
-
- return true;
-}
-} // namespace base64