From 5c7e5cc738b0dbdc76f84c470d52eb778c605225 Mon Sep 17 00:00:00 2001 From: Lucas CHOLLET Date: Fri, 8 Sep 2023 06:30:50 -0400 Subject: [PATCH] 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 | 9 +++-- Ladybird/HelperProcess.cpp | 5 +++ Ladybird/HelperProcess.h | 2 + Ladybird/ImageCodecPlugin.cpp | 38 ++++++++++--------- Ladybird/ImageCodecPlugin.h | 4 ++ Ladybird/ImageDecoder/CMakeLists.txt | 12 ++++++ Ladybird/ImageDecoder/main.cpp | 33 ++++++++++++++++ Ladybird/WebContent/CMakeLists.txt | 2 +- .../Libraries/LibImageDecoderClient/Client.h | 4 +- Userland/Utilities/CMakeLists.txt | 2 +- 10 files changed, 86 insertions(+), 25 deletions(-) create mode 100644 Ladybird/ImageDecoder/CMakeLists.txt create mode 100644 Ladybird/ImageDecoder/main.cpp diff --git a/Ladybird/CMakeLists.txt b/Ladybird/CMakeLists.txt index 6f34387d01c..a5455b02f06 100644 --- a/Ladybird/CMakeLists.txt +++ b/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 diff --git a/Ladybird/HelperProcess.cpp b/Ladybird/HelperProcess.cpp index cef3a5c4ddb..b0b4990ff92 100644 --- a/Ladybird/HelperProcess.cpp +++ b/Ladybird/HelperProcess.cpp @@ -144,6 +144,11 @@ ErrorOr> launch_generic_server_process(ReadonlySpan> launch_image_decoder_process(ReadonlySpan candidate_image_decoder_paths) +{ + return launch_generic_server_process(candidate_image_decoder_paths, ""sv, "ImageDecoder"sv); +} + ErrorOr> launch_request_server_process(ReadonlySpan candidate_request_server_paths, StringView serenity_resource_root) { return launch_generic_server_process(candidate_request_server_paths, serenity_resource_root, "RequestServer"sv); diff --git a/Ladybird/HelperProcess.h b/Ladybird/HelperProcess.h index e46f9602835..1f9e0925c66 100644 --- a/Ladybird/HelperProcess.h +++ b/Ladybird/HelperProcess.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -21,5 +22,6 @@ ErrorOr> launch_web_content_process(Web WebView::IsLayoutTestMode, Ladybird::UseLagomNetworking); +ErrorOr> launch_image_decoder_process(ReadonlySpan candidate_image_decoder_paths); ErrorOr> launch_request_server_process(ReadonlySpan candidate_request_server_paths, StringView serenity_resource_root); ErrorOr> launch_web_socket_process(ReadonlySpan candidate_web_socket_paths, StringView serenity_resource_root); diff --git a/Ladybird/ImageCodecPlugin.cpp b/Ladybird/ImageCodecPlugin.cpp index ba6414f504f..95d8c38e59e 100644 --- a/Ladybird/ImageCodecPlugin.cpp +++ b/Ladybird/ImageCodecPlugin.cpp @@ -6,35 +6,39 @@ */ #include "ImageCodecPlugin.h" +#include "HelperProcess.h" +#include "Utilities.h" #include #include +#include namespace Ladybird { ImageCodecPlugin::~ImageCodecPlugin() = default; -Optional ImageCodecPlugin::decode_image(ReadonlyBytes data) +Optional 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(); + + 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); } - Vector 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(frame.duration) }); - } - - return Web::Platform::DecodedImage { - decoder->is_animated(), - static_cast(decoder->loop_count()), - move(frames), - }; + return decoded_image; } } diff --git a/Ladybird/ImageCodecPlugin.h b/Ladybird/ImageCodecPlugin.h index 4637fb6ce6d..e6774e9041c 100644 --- a/Ladybird/ImageCodecPlugin.h +++ b/Ladybird/ImageCodecPlugin.h @@ -7,6 +7,7 @@ #pragma once +#include #include namespace Ladybird { @@ -17,6 +18,9 @@ public: virtual ~ImageCodecPlugin() override; virtual Optional decode_image(ReadonlyBytes data) override; + +private: + RefPtr m_client; }; } diff --git a/Ladybird/ImageDecoder/CMakeLists.txt b/Ladybird/ImageDecoder/CMakeLists.txt new file mode 100644 index 00000000000..a1eca1f9f9f --- /dev/null +++ b/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) diff --git a/Ladybird/ImageDecoder/main.cpp b/Ladybird/ImageDecoder/main.cpp new file mode 100644 index 00000000000..17af47e8c35 --- /dev/null +++ b/Ladybird/ImageDecoder/main.cpp @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2023, Andrew Kaster + * Copyright (c) 2023, Lucas Chollet + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include + +ErrorOr 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()); + client->set_fd_passing_socket(TRY(Core::LocalSocket::adopt_fd(fd_passing_socket))); + + auto result = event_loop.exec(); + + return result; +} diff --git a/Ladybird/WebContent/CMakeLists.txt b/Ladybird/WebContent/CMakeLists.txt index 6022763009a..7ef55238dbb 100644 --- a/Ladybird/WebContent/CMakeLists.txt +++ b/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) diff --git a/Userland/Libraries/LibImageDecoderClient/Client.h b/Userland/Libraries/LibImageDecoderClient/Client.h index d04ac218731..835a5d61ffb 100644 --- a/Userland/Libraries/LibImageDecoderClient/Client.h +++ b/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); + Optional decode_image(ReadonlyBytes, Optional mime_type = {}); Function on_death; private: - Client(NonnullOwnPtr); - virtual void die() override; }; diff --git a/Userland/Utilities/CMakeLists.txt b/Userland/Utilities/CMakeLists.txt index a33b183088a..cb7168a1a65 100644 --- a/Userland/Utilities/CMakeLists.txt +++ b/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)