From edf01803cd368c1796ec3bdf39f8c9e851fc0e46 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 25 Dec 2020 00:19:06 +0100 Subject: [PATCH] LibGfx: Make all image decoders reject image sizes above 16384 pixels Let's just say no to shenanigans by capping images at 16384 pixels both wide and tall. If a day comes in the future where we need to handle images larger than this, we can deal with it then. --- Libraries/LibGfx/BMPLoader.cpp | 5 +++++ Libraries/LibGfx/GIFLoader.cpp | 5 +++++ Libraries/LibGfx/ImageDecoder.h | 3 +++ Libraries/LibGfx/JPGLoader.cpp | 6 ++++++ Libraries/LibGfx/PBMLoader.cpp | 4 ++-- Libraries/LibGfx/PNGLoader.cpp | 4 ++-- Libraries/LibGfx/PortableImageLoaderCommon.h | 21 ++++++++++++++------ 7 files changed, 38 insertions(+), 10 deletions(-) diff --git a/Libraries/LibGfx/BMPLoader.cpp b/Libraries/LibGfx/BMPLoader.cpp index 636b94b241..6a779ae127 100644 --- a/Libraries/LibGfx/BMPLoader.cpp +++ b/Libraries/LibGfx/BMPLoader.cpp @@ -518,6 +518,11 @@ static bool decode_bmp_core_dib(BMPLoadingContext& context, Streamer& streamer) return false; } + if (static_cast(core.width) > maximum_width_for_decoded_images || static_cast(abs(core.height)) > maximum_height_for_decoded_images) { + dbgln("This BMP is too large for comfort: {}x{}", core.width, abs(core.height)); + return false; + } + auto color_planes = streamer.read_u16(); if (color_planes != 1) { IF_BMP_DEBUG(dbg() << "BMP has an invalid number of color planes: " << color_planes); diff --git a/Libraries/LibGfx/GIFLoader.cpp b/Libraries/LibGfx/GIFLoader.cpp index b27aa05868..d50bd73a60 100644 --- a/Libraries/LibGfx/GIFLoader.cpp +++ b/Libraries/LibGfx/GIFLoader.cpp @@ -424,6 +424,11 @@ static bool load_gif_frame_descriptors(GIFLoadingContext& context) if (stream.handle_any_error()) return false; + if (context.logical_screen.width > maximum_width_for_decoded_images || context.logical_screen.height > maximum_height_for_decoded_images) { + dbgln("This GIF is too large for comfort: {}x{}", context.logical_screen.width, context.logical_screen.height); + return false; + } + u8 gcm_info = 0; stream >> gcm_info; diff --git a/Libraries/LibGfx/ImageDecoder.h b/Libraries/LibGfx/ImageDecoder.h index 7b197e5e19..a054f524a9 100644 --- a/Libraries/LibGfx/ImageDecoder.h +++ b/Libraries/LibGfx/ImageDecoder.h @@ -36,6 +36,9 @@ namespace Gfx { class Bitmap; +static constexpr size_t maximum_width_for_decoded_images = 16384; +static constexpr size_t maximum_height_for_decoded_images = 16384; + struct ImageFrameDescriptor { RefPtr image; int duration { 0 }; diff --git a/Libraries/LibGfx/JPGLoader.cpp b/Libraries/LibGfx/JPGLoader.cpp index 846fc674a5..650227a72b 100644 --- a/Libraries/LibGfx/JPGLoader.cpp +++ b/Libraries/LibGfx/JPGLoader.cpp @@ -776,6 +776,12 @@ static bool read_start_of_frame(InputMemoryStream& stream, JPGLoadingContext& co #endif return false; } + + if (context.frame.width > maximum_width_for_decoded_images || context.frame.height > maximum_height_for_decoded_images) { + dbgln("This JPEG is too large for comfort: {}x{}", context.frame.width, context.frame.height); + return false; + } + set_macroblock_metadata(context); stream >> context.component_count; diff --git a/Libraries/LibGfx/PBMLoader.cpp b/Libraries/LibGfx/PBMLoader.cpp index 487cb9ff02..14694380ea 100644 --- a/Libraries/LibGfx/PBMLoader.cpp +++ b/Libraries/LibGfx/PBMLoader.cpp @@ -60,8 +60,8 @@ struct PBMLoadingContext { State state { State::NotDecoded }; const u8* data { nullptr }; size_t data_size { 0 }; - int width { -1 }; - int height { -1 }; + size_t width { 0 }; + size_t height { 0 }; RefPtr bitmap; }; diff --git a/Libraries/LibGfx/PNGLoader.cpp b/Libraries/LibGfx/PNGLoader.cpp index 49c546e8a2..b5f9657b86 100644 --- a/Libraries/LibGfx/PNGLoader.cpp +++ b/Libraries/LibGfx/PNGLoader.cpp @@ -853,8 +853,8 @@ static bool process_IHDR(ReadonlyBytes data, PNGLoadingContext& context) return false; auto& ihdr = *(const PNG_IHDR*)data.data(); - if (ihdr.width > NumericLimits::max() || ihdr.height > NumericLimits::max()) { - dbgln("PNG has invalid geometry {}x{}", (u32)ihdr.width, (u32)ihdr.height); + if (ihdr.width > maximum_width_for_decoded_images || ihdr.height > maximum_height_for_decoded_images) { + dbgln("This PNG is too large for comfort: {}x{}", (u32)ihdr.width, (u32)ihdr.height); return false; } diff --git a/Libraries/LibGfx/PortableImageLoaderCommon.h b/Libraries/LibGfx/PortableImageLoaderCommon.h index a95c2127c5..4703375246 100644 --- a/Libraries/LibGfx/PortableImageLoaderCommon.h +++ b/Libraries/LibGfx/PortableImageLoaderCommon.h @@ -1,5 +1,6 @@ /* - * Copyright (c) 2020, the SerenityOS developers, Hüseyin ASLITÜRK + * Copyright (c) 2020, Hüseyin Aslıtürk + * Copyright (c) 2020, the SerenityOS developers * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -26,7 +27,6 @@ #pragma once -#include "Streamer.h" #include #include #include @@ -36,6 +36,10 @@ #include #include #include +#include +#include +#include +#include //#define PORTABLE_IMAGE_LOADER_DEBUG @@ -216,8 +220,8 @@ template static void set_adjusted_pixels(TContext& context, const AK::Vector& color_data) { size_t index = 0; - for (int y = 0; y < context.height; ++y) { - for (int x = 0; x < context.width; ++x) { + for (size_t y = 0; y < context.height; ++y) { + for (size_t x = 0; x < context.width; ++x) { Color color = color_data.at(index); if (context.max_val < 255) { color = adjust_color(context.max_val, color); @@ -232,8 +236,8 @@ template static void set_pixels(TContext& context, const AK::Vector& color_data) { size_t index = 0; - for (int y = 0; y < context.height; ++y) { - for (int x = 0; x < context.width; ++x) { + for (size_t y = 0; y < context.height; ++y) { + for (size_t x = 0; x < context.width; ++x) { context.bitmap->set_pixel(x, y, color_data.at(index)); index++; } @@ -267,6 +271,11 @@ static bool decode(TContext& context) if (!read_height(context, streamer)) return false; + if (context.width > maximum_width_for_decoded_images || context.height > maximum_height_for_decoded_images) { + dbgln("This portable network image is too large for comfort: {}x{}", context.width, context.height); + return false; + } + if (!read_white_space(context, streamer)) return false;