From c2a59d18e8119b39f20a2ed22b161cf284b13503 Mon Sep 17 00:00:00 2001 From: karthik Date: Tue, 7 Jul 2026 12:25:18 -0700 Subject: [PATCH] Fix unsafe AdaptiveBase64 decode handling Guard malformed base64 padding and invalid input before decoding. Decode into a temporary vector with bounded push_back writes so malformed data cannot write past the allocated output buffer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Base64Test.cpp | 34 +++++- .../cpp/ObjectModel/AdaptiveBase64Util.cpp | 105 ++++++++++++------ 2 files changed, 100 insertions(+), 39 deletions(-) diff --git a/source/shared/cpp/AdaptiveCardsSharedModel/AdaptiveCardsSharedModelUnitTest/Base64Test.cpp b/source/shared/cpp/AdaptiveCardsSharedModel/AdaptiveCardsSharedModelUnitTest/Base64Test.cpp index b0623556eb..c3700022ba 100644 --- a/source/shared/cpp/AdaptiveCardsSharedModel/AdaptiveCardsSharedModelUnitTest/Base64Test.cpp +++ b/source/shared/cpp/AdaptiveCardsSharedModel/AdaptiveCardsSharedModelUnitTest/Base64Test.cpp @@ -60,14 +60,42 @@ namespace AdaptiveCardsSharedModelUnitTest } } + TEST_METHOD(DecodingUnpaddedTest) + { + std::vector expectedDecodedData{ "f", "fo", "foobar" }; + std::vector encodedData{ "Zg", "Zm8", "Zm9vYmFy" }; + + for (size_t i{}; i < encodedData.size(); ++i) + { + Assert::IsTrue(ContainSameCharacters(expectedDecodedData[i], + AdaptiveBase64Util::Decode(std::string(encodedData[i].begin(), encodedData[i].end())))); + } + } + TEST_METHOD(FailToDecodeTest) { - std::vector badUri{ "foo_bar", "foo(bar)", "foo-bar", "foo*bar", "foo\"bar", "foo&bar", "foo^bar", "foo#bar", "foo@bar", "foo!bar" }; + std::vector badUri{ + "foo_bar", + "foo(bar)", + "foo-bar", + "foo*bar", + "foo\"bar", + "foo&bar", + "foo^bar", + "foo#bar", + "foo@bar", + "foo!bar", + "A", + "====", + "AAAA====", + "Zm=9", + "Zg=", + "Zg===", + }; - // If it crashes in any case the test will fail for (const auto& uri : badUri) { - AdaptiveBase64Util::Decode(uri); + Assert::IsTrue(AdaptiveBase64Util::Decode(uri).empty()); } } diff --git a/source/shared/cpp/ObjectModel/AdaptiveBase64Util.cpp b/source/shared/cpp/ObjectModel/AdaptiveBase64Util.cpp index 2897a63bc2..947b751dd0 100644 --- a/source/shared/cpp/ObjectModel/AdaptiveBase64Util.cpp +++ b/source/shared/cpp/ObjectModel/AdaptiveBase64Util.cpp @@ -55,28 +55,41 @@ const unsigned char c_base64DecodeTable[] = { size_t AdaptiveBase64Util::DecodedLength(const char* in, size_t in_length) { - int numEq{}; + if (in == nullptr || in_length == 0) + { + return 0; + } - const char* in_end{in + in_length}; - while (*--in_end == '=') + size_t numEq{}; + while (numEq < in_length && in[in_length - 1 - numEq] == '=') { ++numEq; } - return ((6 * in_length) / 8) - numEq; -} - -size_t AdaptiveBase64Util::DecodedLength(const std::string& in) -{ - size_t numEq{}; - size_t n{in.size()}; + if (numEq > 2) + { + numEq = 2; + } - for (auto it = in.rbegin(); (numEq < n) && (*it == '='); ++it) + size_t decodedLength = (in_length / 4) * 3; + switch (in_length % 4) { - ++numEq; + case 2: + decodedLength += 1; + break; + case 3: + decodedLength += 2; + break; + default: + break; } - return ((6 * n) / 8) - numEq; + return decodedLength >= numEq ? decodedLength - numEq : 0; +} + +size_t AdaptiveBase64Util::DecodedLength(const std::string& in) +{ + return DecodedLength(in.data(), in.size()); } size_t AdaptiveBase64Util::EncodedLength(size_t length) @@ -174,35 +187,51 @@ bool AdaptiveBase64Util::Encode(const std::vector& in, std::string* out) bool AdaptiveBase64Util::Decode(const std::string& in, std::vector* out) { - size_t input_len{in.size()}; - auto input = in.begin(); + if (out == nullptr) + { + return false; + } - out->resize(DecodedLength(in)); + out->clear(); + if (in.empty()) + { + return true; + } + + size_t numEq{}; + while (numEq < in.size() && in[in.size() - 1 - numEq] == '=') + { + ++numEq; + } + + if (numEq > 2 || (numEq > 0 && in.size() % 4 != 0) || in.size() % 4 == 1) + { + return false; + } + + const size_t dataLength = in.size() - numEq; + std::vector decoded; + decoded.reserve(DecodedLength(in)); int i{}; - size_t dec_len = 0; - unsigned char a3[3]; - unsigned char a4[4]; - while (input_len--) + unsigned char a3[3]{}; + unsigned char a4[4]{}; + for (size_t inputIndex{}; inputIndex < dataLength; ++inputIndex) { - if (*input == '=') + const unsigned char decodedValue = b64_lookup(static_cast(in[inputIndex])); + if (decodedValue == 0xFF) { - break; + return false; } - a4[i++] = *(input++); + a4[i++] = decodedValue; if (i == 4) { - for (i = 0; i < 4; ++i) - { - a4[i] = b64_lookup(a4[i]); - } - a4_to_a3(a3, a4); for (i = 0; i < 3; ++i) { - (*out)[dec_len++] = a3[i]; + decoded.push_back(static_cast(a3[i])); } i = 0; @@ -211,31 +240,35 @@ bool AdaptiveBase64Util::Decode(const std::string& in, std::vector* out) if (i) { - for (int j{i}; j < 4; ++j) + if (i == 1) { - a4[j] = '\0'; + return false; } - for (int j{}; j < 4; j++) + for (int j{i}; j < 4; ++j) { - a4[j] = b64_lookup(a4[j]); + a4[j] = 0; } a4_to_a3(a3, a4); for (int j{}; j < i - 1; ++j) { - (*out)[dec_len++] = a3[j]; + decoded.push_back(static_cast(a3[j])); } } - return (dec_len == out->size()); + out->swap(decoded); + return true; } std::vector AdaptiveBase64Util::Decode(const std::string& encodedBase64) { std::vector decodedString; - Decode(encodedBase64, &decodedString); + if (!Decode(encodedBase64, &decodedString)) + { + decodedString.clear(); + } return decodedString; }