Ladybird: Decode images out of process

This patch brings a service to handle image decompression. With it comes
security enhancement due to the process boundary. Indeed, consequences
of a potential attack is reduced as only the decoder will crash without
perturbing the WebContent process.
It also allows us to display pages containing images that we claim to
support but still make us crash, like for not-finished-yet decoders.

As an example, we can now load https://jpegxl.info/jxl-art.html without
crashing the WebContent process.

Ladybird/CMakeLists.txt

@@ -178,7 +178,7 @@ target_sources(ladybird PUBLIC FILE_SET ladybird TYPE HEADERS
     BASE_DIRS ${SERENITY_SOURCE_DIR}
     FILES ${LADYBIRD_HEADERS}
 )
-target_link_libraries(ladybird PRIVATE LibCore LibFileSystem LibGfx LibGUI LibIPC LibJS LibMain LibWeb LibWebView LibProtocol)
+target_link_libraries(ladybird PRIVATE LibCore LibFileSystem LibGfx LibGUI LibImageDecoderClient LibIPC LibJS LibMain LibWeb LibWebView LibProtocol)
 
 target_include_directories(ladybird PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
 target_include_directories(ladybird PRIVATE ${SERENITY_SOURCE_DIR}/Userland/)
@@ -192,7 +192,7 @@ add_executable(headless-browser
     Utilities.cpp)
 
 target_include_directories(headless-browser PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
-target_link_libraries(headless-browser PRIVATE LibWeb LibWebView LibWebSocket LibCrypto LibFileSystem LibGemini LibHTTP LibJS LibGfx LibMain LibTLS LibIPC LibDiff LibProtocol)
+target_link_libraries(headless-browser PRIVATE LibWeb LibWebView LibWebSocket LibCrypto LibFileSystem LibGemini LibHTTP LibImageDecoderClient LibJS LibGfx LibMain LibTLS LibIPC LibDiff LibProtocol)
 
 if (ANDROID)
     include(cmake/AndroidExtras.cmake)
@@ -209,12 +209,13 @@ add_custom_target(debug${LADYBIRD_CUSTOM_TARGET_SUFFIX}
     USES_TERMINAL
 )
 
+add_subdirectory(ImageDecoder)
+add_subdirectory(RequestServer)
 add_subdirectory(SQLServer)
 add_subdirectory(WebContent)
 add_subdirectory(WebDriver)
 add_subdirectory(WebSocket)
-add_subdirectory(RequestServer)
-add_dependencies(ladybird SQLServer WebContent WebDriver WebSocketServer RequestServer headless-browser)
+add_dependencies(ladybird ImageDecoder RequestServer SQLServer WebContent WebDriver WebSocketServer headless-browser)
 
 function(create_ladybird_bundle target_name)
     set_target_properties(${target_name} PROPERTIES

Ladybird/HelperProcess.cpp

@@ -144,6 +144,11 @@ ErrorOr<NonnullRefPtr<Client>> launch_generic_server_process(ReadonlySpan<String
     return new_client;
 }
 
+ErrorOr<NonnullRefPtr<ImageDecoderClient::Client>> launch_image_decoder_process(ReadonlySpan<String> candidate_image_decoder_paths)
+{
+    return launch_generic_server_process<ImageDecoderClient::Client>(candidate_image_decoder_paths, ""sv, "ImageDecoder"sv);
+}
+
 ErrorOr<NonnullRefPtr<Protocol::RequestClient>> launch_request_server_process(ReadonlySpan<String> candidate_request_server_paths, StringView serenity_resource_root)
 {
     return launch_generic_server_process<Protocol::RequestClient>(candidate_request_server_paths, serenity_resource_root, "RequestServer"sv);

Ladybird/HelperProcess.h

@@ -10,6 +10,7 @@
 #include <AK/Error.h>
 #include <AK/Span.h>
 #include <AK/StringView.h>
+#include <LibImageDecoderClient/Client.h>
 #include <LibProtocol/RequestClient.h>
 #include <LibProtocol/WebSocketClient.h>
 #include <LibWebView/ViewImplementation.h>
@@ -21,5 +22,6 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(Web
     WebView::IsLayoutTestMode,
     Ladybird::UseLagomNetworking);
 
+ErrorOr<NonnullRefPtr<ImageDecoderClient::Client>> launch_image_decoder_process(ReadonlySpan<String> candidate_image_decoder_paths);
 ErrorOr<NonnullRefPtr<Protocol::RequestClient>> launch_request_server_process(ReadonlySpan<String> candidate_request_server_paths, StringView serenity_resource_root);
 ErrorOr<NonnullRefPtr<Protocol::WebSocketClient>> launch_web_socket_process(ReadonlySpan<String> candidate_web_socket_paths, StringView serenity_resource_root);

Ladybird/ImageCodecPlugin.cpp

@@ -6,35 +6,39 @@
  */
 
 #include "ImageCodecPlugin.h"
+#include "HelperProcess.h"
+#include "Utilities.h"
 #include <LibGfx/Bitmap.h>
 #include <LibGfx/ImageFormats/ImageDecoder.h>
+#include <LibImageDecoderClient/Client.h>
 
 namespace Ladybird {
 
 ImageCodecPlugin::~ImageCodecPlugin() = default;
 
-Optional<Web::Platform::DecodedImage> ImageCodecPlugin::decode_image(ReadonlyBytes data)
+Optional<Web::Platform::DecodedImage> ImageCodecPlugin::decode_image(ReadonlyBytes bytes)
 {
-    auto decoder = Gfx::ImageDecoder::try_create_for_raw_bytes(data);
+    if (!m_client) {
+        auto candidate_image_decoder_paths = get_paths_for_helper_process("ImageDecoder"sv).release_value_but_fixme_should_propagate_errors();
+        m_client = launch_image_decoder_process(candidate_image_decoder_paths).release_value_but_fixme_should_propagate_errors();
+        m_client->on_death = [&] {
+            m_client = nullptr;
+        };
+    }
 
-    if (!decoder || !decoder->frame_count()) {
+    auto result_or_empty = m_client->decode_image(bytes);
+    if (!result_or_empty.has_value())
         return {};
-    }
+    auto result = result_or_empty.release_value();
 
-    Vector<Web::Platform::Frame> frames;
-    for (size_t i = 0; i < decoder->frame_count(); ++i) {
-        auto frame_or_error = decoder->frame(i);
-        if (frame_or_error.is_error())
-            return {};
-        auto frame = frame_or_error.release_value();
-        frames.append({ move(frame.image), static_cast<size_t>(frame.duration) });
+    Web::Platform::DecodedImage decoded_image;
+    decoded_image.is_animated = result.is_animated;
+    decoded_image.loop_count = result.loop_count;
+    for (auto const& frame : result.frames) {
+        decoded_image.frames.empend(move(frame.bitmap), frame.duration);
     }
 
-    return Web::Platform::DecodedImage {
-        decoder->is_animated(),
-        static_cast<u32>(decoder->loop_count()),
-        move(frames),
-    };
+    return decoded_image;
 }
 
 }

Ladybird/ImageCodecPlugin.h

@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <LibImageDecoderClient/Client.h>
 #include <LibWeb/Platform/ImageCodecPlugin.h>
 
 namespace Ladybird {
@@ -17,6 +18,9 @@ public:
     virtual ~ImageCodecPlugin() override;
 
     virtual Optional<Web::Platform::DecodedImage> decode_image(ReadonlyBytes data) override;
+
+private:
+    RefPtr<ImageDecoderClient::Client> m_client;
 };
 
 }

Ladybird/ImageDecoder/CMakeLists.txt

@@ -0,0 +1,12 @@
+set(IMAGE_DECODER_SOURCE_DIR ${SERENITY_SOURCE_DIR}/Userland/Services/ImageDecoder)
+
+set(IMAGE_DECODER_SOURCES
+    ${IMAGE_DECODER_SOURCE_DIR}/ConnectionFromClient.cpp
+    main.cpp
+)
+
+add_executable(ImageDecoder ${IMAGE_DECODER_SOURCES})
+
+target_include_directories(ImageDecoder PRIVATE ${SERENITY_SOURCE_DIR}/Userland/Services/)
+target_include_directories(ImageDecoder PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/..)
+target_link_libraries(ImageDecoder PRIVATE LibCore LibGfx LibIPC LibImageDecoderClient LibMain)

Ladybird/ImageDecoder/main.cpp

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2023, Andrew Kaster <akaster@serenityos.org>
+ * Copyright (c) 2023, Lucas Chollet <lucas.chollet@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <ImageDecoder/ConnectionFromClient.h>
+#include <LibCore/ArgsParser.h>
+#include <LibCore/EventLoop.h>
+#include <LibIPC/SingleServer.h>
+#include <LibMain/Main.h>
+
+ErrorOr<int> serenity_main(Main::Arguments arguments)
+{
+    int fd_passing_socket { -1 };
+    StringView serenity_resource_root;
+
+    Core::ArgsParser args_parser;
+    args_parser.add_option(fd_passing_socket, "File descriptor of the fd passing socket", "fd-passing-socket", 'c', "fd-passing-socket");
+    args_parser.add_option(serenity_resource_root, "Absolute path to directory for serenity resources", "serenity-resource-root", 'r', "serenity-resource-root");
+    args_parser.parse(arguments);
+
+    Core::EventLoop event_loop;
+
+    auto client = TRY(IPC::take_over_accepted_client_from_system_server<ImageDecoder::ConnectionFromClient>());
+    client->set_fd_passing_socket(TRY(Core::LocalSocket::adopt_fd(fd_passing_socket)));
+
+    auto result = event_loop.exec();
+
+    return result;
+}

Ladybird/WebContent/CMakeLists.txt

@@ -68,7 +68,7 @@ endif()
 
 target_include_directories(WebContent PRIVATE ${SERENITY_SOURCE_DIR}/Userland/Services/)
 target_include_directories(WebContent PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/..)
-target_link_libraries(WebContent PRIVATE LibAudio LibCore LibFileSystem LibGfx LibIPC LibJS LibMain LibWeb LibWebSocket LibProtocol LibWebView)
+target_link_libraries(WebContent PRIVATE LibAudio LibCore LibFileSystem LibGfx LibImageDecoderClient LibIPC LibJS LibMain LibWeb LibWebSocket LibProtocol LibWebView)
 
 if (HAVE_PULSEAUDIO)
     target_compile_definitions(WebContent PRIVATE HAVE_PULSEAUDIO=1)

Userland/Libraries/LibImageDecoderClient/Client.h

@@ -30,13 +30,13 @@ class Client final
     IPC_CLIENT_CONNECTION(Client, "/tmp/session/%sid/portal/image"sv);
 
 public:
+    Client(NonnullOwnPtr<Core::LocalSocket>);
+
     Optional<DecodedImage> decode_image(ReadonlyBytes, Optional<DeprecatedString> mime_type = {});
 
     Function<void()> on_death;
 
 private:
-    Client(NonnullOwnPtr<Core::LocalSocket>);
-
     virtual void die() override;
 };
 

Userland/Utilities/CMakeLists.txt

@@ -99,7 +99,7 @@ target_link_libraries(gml-format PRIVATE LibGUI)
 target_link_libraries(grep PRIVATE LibFileSystem LibRegex)
 target_link_libraries(gunzip PRIVATE LibCompress)
 target_link_libraries(gzip PRIVATE LibCompress)
-target_link_libraries(headless-browser PRIVATE LibCrypto LibFileSystem LibGemini LibGfx LibHTTP LibTLS LibWeb LibWebView LibWebSocket LibIPC LibJS LibDiff)
+target_link_libraries(headless-browser PRIVATE LibCrypto LibFileSystem LibGemini LibGfx LibHTTP LibImageDecoderClient LibTLS LibWeb LibWebView LibWebSocket LibIPC LibJS LibDiff)
 target_link_libraries(icc PRIVATE LibGfx LibVideo)
 target_link_libraries(image PRIVATE LibGfx)
 target_link_libraries(image2bin PRIVATE LibGfx)