AK+Everywhere: Make Base64 decoding fallible

AK/Base64.cpp

@@ -6,10 +6,7 @@
 
 #include <AK/Array.h>
 #include <AK/Base64.h>
-#include <AK/ByteBuffer.h>
-#include <AK/String.h>
 #include <AK/StringBuilder.h>
-#include <AK/StringView.h>
 #include <AK/Types.h>
 #include <AK/Vector.h>
 
@@ -33,7 +30,8 @@ static constexpr auto make_alphabet()
 static constexpr auto make_lookup_table()
 {
     constexpr auto alphabet = make_alphabet();
-    Array<u8, 256> table {};
+    Array<i16, 256> table;
+    table.fill(-1);
     for (size_t i = 0; i < alphabet.size(); ++i) {
         table[alphabet[i]] = i;
     }
@@ -50,19 +48,31 @@ size_t calculate_base64_encoded_length(ReadonlyBytes input)
     return ((4 * input.size() / 3) + 3) & ~3;
 }
 
-ByteBuffer decode_base64(const StringView& input)
+Optional<ByteBuffer> decode_base64(const StringView& input)
 {
-    auto get = [&](const size_t offset, bool* is_padding = nullptr) -> u8 {
+    auto get = [&](const size_t offset, bool* is_padding) -> Optional<u8> {
         constexpr auto table = make_lookup_table();
         if (offset >= input.length())
             return 0;
         if (input[offset] == '=') {
-            if (is_padding)
+            if (!is_padding)
-                *is_padding = true;
+                return {};
+            *is_padding = true;
             return 0;
         }
-        return table[static_cast<unsigned char>(input[offset])];
+        i16 result = table[static_cast<unsigned char>(input[offset])];
+        if (result < 0)
+            return {};
+        VERIFY(result < 256);
+        return { result };
     };
+#define TRY_GET(index, is_padding)                       \
+    ({                                                   \
+        auto _temporary_result = get(index, is_padding); \
+        if (!_temporary_result.has_value())              \
+            return {};                                   \
+        _temporary_result.value();                       \
+    })
 
     Vector<u8> output;
     output.ensure_capacity(calculate_base64_decoded_length(input));
@@ -71,10 +81,10 @@ ByteBuffer decode_base64(const StringView& input)
         bool in2_is_padding = false;
         bool in3_is_padding = false;
 
-        const u8 in0 = get(i);
+        const u8 in0 = TRY_GET(i, nullptr);
-        const u8 in1 = get(i + 1);
+        const u8 in1 = TRY_GET(i + 1, nullptr);
-        const u8 in2 = get(i + 2, &in2_is_padding);
+        const u8 in2 = TRY_GET(i + 2, &in2_is_padding);
-        const u8 in3 = get(i + 3, &in3_is_padding);
+        const u8 in3 = TRY_GET(i + 3, &in3_is_padding);
 
         const u8 out0 = (in0 << 2) | ((in1 >> 4) & 3);
         const u8 out1 = ((in1 & 0xf) << 4) | ((in2 >> 2) & 0xf);
@@ -87,8 +97,7 @@ ByteBuffer decode_base64(const StringView& input)
             output.append(out2);
     }
 
-    // FIXME: Handle OOM failure.
+    return ByteBuffer::copy(output);
-    return ByteBuffer::copy(output).release_value();
 }
 
 String encode_base64(ReadonlyBytes input)

AK/Base64.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <AK/ByteBuffer.h>
+#include <AK/Optional.h>
 #include <AK/String.h>
 #include <AK/StringView.h>
 
@@ -16,7 +17,7 @@ size_t calculate_base64_decoded_length(const StringView&);
 
 size_t calculate_base64_encoded_length(ReadonlyBytes);
 
-ByteBuffer decode_base64(const StringView&);
+Optional<ByteBuffer> decode_base64(const StringView&);
 
 String encode_base64(ReadonlyBytes);
 

Tests/AK/TestBase64.cpp

@@ -13,7 +13,9 @@
 TEST_CASE(test_decode)
 {
     auto decode_equal = [&](const char* input, const char* expected) {
-        auto decoded = decode_base64(StringView(input));
+        auto decoded_option = decode_base64(StringView(input));
+        EXPECT(decoded_option.has_value());
+        auto decoded = decoded_option.value();
         EXPECT(String::copy(decoded) == String(expected));
         EXPECT(StringView(expected).length() <= calculate_base64_decoded_length(StringView(input).bytes()));
     };
@@ -27,12 +29,12 @@ TEST_CASE(test_decode)
     decode_equal("Zm9vYmFy", "foobar");
 }
 
-TEST_CASE(test_decode_nocrash)
+TEST_CASE(test_decode_invalid)
 {
-    // Any output is fine, we only check that we don't crash here.
+    EXPECT(!decode_base64(StringView("asdf\xffqwe")).has_value());
-    decode_base64(StringView("asdf\xffqwer"));
+    EXPECT(!decode_base64(StringView("asdf\x80qwe")).has_value());
-    decode_base64(StringView("asdf\x80qwer"));
+    EXPECT(!decode_base64(StringView("asdf:qwe")).has_value());
-    // TODO: Handle decoding failure.
+    EXPECT(!decode_base64(StringView("asdf=qwe")).has_value());
 }
 
 TEST_CASE(test_encode)

Userland/Applications/Mail/MailWidget.cpp

@@ -493,7 +493,7 @@ void MailWidget::selected_email_to_load()
     if (selected_alternative_encoding.equals_ignoring_case("7bit") || selected_alternative_encoding.equals_ignoring_case("8bit")) {
         decoded_data = encoded_data;
     } else if (selected_alternative_encoding.equals_ignoring_case("base64")) {
-        decoded_data = decode_base64(encoded_data);
+        decoded_data = decode_base64(encoded_data).value_or(ByteBuffer());
     } else if (selected_alternative_encoding.equals_ignoring_case("quoted-printable")) {
         decoded_data = IMAP::decode_quoted_printable(encoded_data);
     } else {

Userland/Applications/PixelPaint/Image.cpp

@@ -100,8 +100,10 @@ Result<NonnullRefPtr<Image>, String> Image::try_create_from_pixel_paint_json(Jso
 
         auto bitmap_base64_encoded = layer_object.get("bitmap").as_string();
         auto bitmap_data = decode_base64(bitmap_base64_encoded);
+        if (!bitmap_data.has_value())
+            return String { "Base64 decode failed"sv };
 
-        auto bitmap = try_decode_bitmap(bitmap_data);
+        auto bitmap = try_decode_bitmap(bitmap_data.value());
         if (!bitmap)
             return String { "Layer bitmap decode failed"sv };
 

Userland/Libraries/LibCrypto/ASN1/PEM.cpp

@@ -35,7 +35,11 @@ ByteBuffer decode_pem(ReadonlyBytes data)
                 break;
             }
             auto b64decoded = decode_base64(lexer.consume_line().trim_whitespace(TrimMode::Right));
-            if (!decoded.try_append(b64decoded.data(), b64decoded.size())) {
+            if (!b64decoded.has_value()) {
+                dbgln("Failed to decode PEM, likely bad Base64");
+                return {};
+            }
+            if (!decoded.try_append(b64decoded.value().data(), b64decoded.value().size())) {
                 dbgln("Failed to decode PEM, likely OOM condition");
                 return {};
             }

Userland/Libraries/LibHTTP/HttpRequest.cpp

@@ -197,7 +197,10 @@ Optional<HttpRequest::BasicAuthenticationCredentials> HttpRequest::parse_http_ba
     auto token = value.substring_view(6);
     if (token.is_empty())
         return {};
-    auto decoded_token = String::copy(decode_base64(token));
+    auto decoded_token_bb = decode_base64(token);
+    if (!decoded_token_bb.has_value())
+        return {};
+    auto decoded_token = String::copy(decoded_token_bb.value());
     auto colon_index = decoded_token.find(':');
     if (!colon_index.has_value())
         return {};

Userland/Libraries/LibWeb/Bindings/WindowObject.cpp

@@ -386,11 +386,15 @@ JS_DEFINE_OLD_NATIVE_FUNCTION(WindowObject::atob)
     }
     auto string = TRY_OR_DISCARD(vm.argument(0).to_string(global_object));
     auto decoded = decode_base64(StringView(string));
+    if (!decoded.has_value()) {
+        vm.throw_exception<JS::TypeError>(global_object, JS::ErrorType::InvalidFormat, "Base64");
+        return {};
+    }
 
     // decode_base64() returns a byte string. LibJS uses UTF-8 for strings. Use Latin1Decoder to convert bytes 128-255 to UTF-8.
     auto decoder = TextCodec::decoder_for("windows-1252");
     VERIFY(decoder);
-    return JS::js_string(vm, decoder->to_utf8(decoded));
+    return JS::js_string(vm, decoder->to_utf8(decoded.value()));
 }
 
 JS_DEFINE_OLD_NATIVE_FUNCTION(WindowObject::btoa)

Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp

@@ -153,10 +153,18 @@ void ResourceLoader::load(LoadRequest& request, Function<void(ReadonlyBytes, con
             url.data_payload());
 
         ByteBuffer data;
-        if (url.data_payload_is_base64())
+        if (url.data_payload_is_base64()) {
-            data = decode_base64(url.data_payload());
+            auto data_maybe = decode_base64(url.data_payload());
-        else
+            if (!data_maybe.has_value()) {
+                auto error_message = "Base64 data contains an invalid character"sv;
+                log_failure(request, error_message);
+                error_callback(error_message, {});
+                return;
+            }
+            data = data_maybe.value();
+        } else {
             data = url.data_payload().to_byte_buffer();
+        }
 
         log_success(request);
         deferred_invoke([data = move(data), success_callback = move(success_callback)] {

Userland/Utilities/base64.cpp

@@ -51,7 +51,11 @@ int main(int argc, char** argv)
 
     if (decode) {
         auto decoded = decode_base64(StringView(buffer));
-        fwrite(decoded.data(), sizeof(u8), decoded.size(), stdout);
+        if (!decoded.has_value()) {
+            warnln("base64: invalid input");
+            return 1;
+        }
+        fwrite(decoded.value().data(), sizeof(u8), decoded.value().size(), stdout);
         return 0;
     }
 

Userland/Utilities/telws.cpp

@@ -113,7 +113,11 @@ int main(int argc, char** argv)
                 }
                 auto base64_data = line.substring(8);
                 auto buffer = decode_base64(base64_data);
-                socket->send(buffer, false);
+                if (buffer.has_value()) {
+                    socket->send(buffer.value(), false);
+                } else {
+                    outln("Could not send message : Base64 string contains an invalid character.");
+                }
                 continue;
             }
             if (line == ".exit") {