diff --git a/sdk/core/azure-core/src/base64.cpp b/sdk/core/azure-core/src/base64.cpp index c0b7b68166..4d7eec5d02 100644 --- a/sdk/core/azure-core/src/base64.cpp +++ b/sdk/core/azure-core/src/base64.cpp @@ -346,16 +346,28 @@ std::string Base64Encode(uint8_t const* const data, size_t length) int32_t Base64Decode(const char* encodedBytes) { - int32_t i0 = encodedBytes[0]; - int32_t i1 = encodedBytes[1]; - int32_t i2 = encodedBytes[2]; - int32_t i3 = encodedBytes[3]; + // Read each input byte as unsigned so that bytes >= 0x80 do not sign-extend to a + // negative value, which would index Base64DecodeArray out of bounds. Invalid bytes + // map to the table's -1 sentinel, handled by the caller's `result < 0` check. + int32_t i0 = static_cast(encodedBytes[0]); + int32_t i1 = static_cast(encodedBytes[1]); + int32_t i2 = static_cast(encodedBytes[2]); + int32_t i3 = static_cast(encodedBytes[3]); i0 = Base64DecodeArray[i0]; i1 = Base64DecodeArray[i1]; i2 = Base64DecodeArray[i2]; i3 = Base64DecodeArray[i3]; + // An invalid character maps to the -1 sentinel. Bail out before the shifts: + // left-shifting a negative value is undefined behavior prior to C++20, which + // sanitizer builds (-fsanitize=undefined -fno-sanitize-recover=all) abort on. + // The caller rejects the negative return value. + if ((i0 | i1 | i2 | i3) < 0) + { + return -1; + } + i0 <<= 18; i1 <<= 12; i2 <<= 6; @@ -418,14 +430,25 @@ std::vector Base64Decode(const std::string& text) // We are guaranteed to have an input with at least 4 bytes at this point, with a size that is a // multiple of 4. - int64_t i0 = inputPtr[inputSize - 4]; - int64_t i1 = inputPtr[inputSize - 3]; - int64_t i2 = inputPtr[inputSize - 2]; - int64_t i3 = inputPtr[inputSize - 1]; + // Read each input byte as unsigned so that bytes >= 0x80 do not sign-extend to a negative + // value, which would index Base64DecodeArray out of bounds. Invalid bytes map to the table's + // -1 sentinel, handled by the `i0 < 0` checks below. + int64_t i0 = static_cast(inputPtr[inputSize - 4]); + int64_t i1 = static_cast(inputPtr[inputSize - 3]); + int64_t i2 = static_cast(inputPtr[inputSize - 2]); + int64_t i3 = static_cast(inputPtr[inputSize - 1]); i0 = Base64DecodeArray[i0]; i1 = Base64DecodeArray[i1]; + // An invalid character maps to the -1 sentinel. Reject it before the shifts below: + // left-shifting a negative value is undefined behavior prior to C++20, which sanitizer + // builds (-fsanitize=undefined -fno-sanitize-recover=all) abort on. + if ((i0 | i1) < 0) + { + throw std::runtime_error("Unexpected character in Base64 encoded string"); + } + i0 <<= 18; i1 <<= 12; @@ -436,40 +459,36 @@ std::vector Base64Decode(const std::string& text) i2 = Base64DecodeArray[i2]; i3 = Base64DecodeArray[i3]; + if ((i2 | i3) < 0) + { + throw std::runtime_error("Unexpected character in Base64 encoded string"); + } + i2 <<= 6; i0 |= i3; i0 |= i2; - if (i0 < 0) - { - throw std::runtime_error("Unexpected character in Base64 encoded string"); - } - Base64WriteThreeLowOrderBytes(destinationPtr, i0); } else if (i2 != EncodingPad) { i2 = Base64DecodeArray[i2]; - i2 <<= 6; - - i0 |= i2; - if (i0 < 0) + if (i2 < 0) { throw std::runtime_error("Unexpected character in Base64 encoded string"); } + i2 <<= 6; + + i0 |= i2; + destinationPtr[1] = static_cast(i0 >> 8); destinationPtr[0] = static_cast(i0 >> 16); } else { - if (i0 < 0) - { - throw std::runtime_error("Unexpected character in Base64 encoded string"); - } - destinationPtr[0] = static_cast(i0 >> 16); } diff --git a/sdk/core/azure-core/test/ut/base64_test.cpp b/sdk/core/azure-core/test/ut/base64_test.cpp index 84a9456437..d3662987f8 100644 --- a/sdk/core/azure-core/test/ut/base64_test.cpp +++ b/sdk/core/azure-core/test/ut/base64_test.cpp @@ -184,5 +184,47 @@ TEST(Base64, InvalidDecode) EXPECT_THROW(Convert::Base64Decode("AD=====EF"), std::runtime_error); EXPECT_THROW(Convert::Base64Decode("AD======EF"), std::runtime_error); + // Invalid bytes map to the decode table's -1 sentinel, which must be rejected before + // any left shift: shifting a negative value is undefined behavior prior to C++20, so + // sanitizer builds (-fsanitize=undefined -fno-sanitize-recover=all) abort otherwise. + // Two distinct defects are covered: + // 1. bytes >= 0x80 must not sign-extend to a negative index (out-of-bounds read); + // 2. the resulting -1 sentinel must not be left-shifted (undefined behavior). + // std::string{...} avoids hex-escape ambiguity in the inputs. + for (char invalid : {'\x80', '\xC0', '\xFF', '\x01', '\x7F'}) + { + // Tail path: invalid byte in each of the four positions of a single group. + EXPECT_THROW(Convert::Base64Decode(std::string{invalid, 'A', 'A', 'A'}), std::runtime_error); + EXPECT_THROW(Convert::Base64Decode(std::string{'A', invalid, 'A', 'A'}), std::runtime_error); + EXPECT_THROW(Convert::Base64Decode(std::string{'A', 'A', invalid, 'A'}), std::runtime_error); + EXPECT_THROW(Convert::Base64Decode(std::string{'A', 'A', 'A', invalid}), std::runtime_error); + // Loop path: invalid byte in each of the eight positions of two groups (the first + // group is decoded through the int32_t Base64Decode(const char*) helper). + EXPECT_THROW( + Convert::Base64Decode(std::string{invalid, 'A', 'A', 'A', 'A', 'A', 'A', 'A'}), + std::runtime_error); + EXPECT_THROW( + Convert::Base64Decode(std::string{'A', invalid, 'A', 'A', 'A', 'A', 'A', 'A'}), + std::runtime_error); + EXPECT_THROW( + Convert::Base64Decode(std::string{'A', 'A', invalid, 'A', 'A', 'A', 'A', 'A'}), + std::runtime_error); + EXPECT_THROW( + Convert::Base64Decode(std::string{'A', 'A', 'A', invalid, 'A', 'A', 'A', 'A'}), + std::runtime_error); + EXPECT_THROW( + Convert::Base64Decode(std::string{'A', 'A', 'A', 'A', invalid, 'A', 'A', 'A'}), + std::runtime_error); + EXPECT_THROW( + Convert::Base64Decode(std::string{'A', 'A', 'A', 'A', 'A', invalid, 'A', 'A'}), + std::runtime_error); + EXPECT_THROW( + Convert::Base64Decode(std::string{'A', 'A', 'A', 'A', 'A', 'A', invalid, 'A'}), + std::runtime_error); + EXPECT_THROW( + Convert::Base64Decode(std::string{'A', 'A', 'A', 'A', 'A', 'A', 'A', invalid}), + std::runtime_error); + } + // cspell::enable }