LibGfx: Reject undersized backing storage when creating bitmaps
Problem: A borked ImageDecoder could send a BitmapSequence over IPC with metadata for a (large) bitmap while shipping a too-small backing buffer. Decoding it produced a Gfx::Bitmap that reported the (large) geometry but pointed at the too-small buffer — making the first write go OOB. Cause: BitmapSequence decode reads size_in_bytes and the bitmap geometry as independent fields, and only checked if size_in_bytes matched the transferred buffer size — never that either is consistent with the geometry. The single-frame fast path then handed the buffer to Bitmap::create_with_anonymous_buffer with no verification. Fix: Make the two bitmap factories that take externally-provided storage enforce that it covers the geometry. create_with_anonymous_buffer now fails with buffers smaller than the minimum expected size_in_bytes — and create_with_raw_data similarly rejects data too small for the geometry. Fixes https://github.com/LadybirdBrowser/ladybird/issues/10036
This commit is contained in:
parent
e623b3e216
commit
8e2eee7654
3 changed files with 84 additions and 0 deletions
|
|
@ -138,6 +138,9 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::create_with_anonymous_buffer(BitmapFormat
|
|||
if (size_would_overflow(format, size))
|
||||
return Error::from_string_literal("Gfx::Bitmap::create_with_anonymous_buffer size overflow");
|
||||
|
||||
if (buffer.size() < size_in_bytes(minimum_pitch(size.width(), format), size.height()))
|
||||
return Error::from_string_literal("Gfx::Bitmap::create_with_anonymous_buffer buffer too small for size");
|
||||
|
||||
return adopt_nonnull_ref_or_enomem(new (nothrow) Bitmap(format, alpha_type, move(buffer), size));
|
||||
}
|
||||
|
||||
|
|
@ -146,6 +149,9 @@ ErrorOr<NonnullRefPtr<Bitmap>> Bitmap::create_with_raw_data(BitmapFormat format,
|
|||
if (size_would_overflow(format, size))
|
||||
return Error::from_string_literal("Gfx::Bitmap::create_with_raw_data size overflow");
|
||||
|
||||
if (raw_data.size() < size_in_bytes(minimum_pitch(size.width(), format), size.height()))
|
||||
return Error::from_string_literal("Gfx::Bitmap::create_with_raw_data data too small for size");
|
||||
|
||||
auto backing_store = TRY(Bitmap::allocate_backing_store(format, size, InitializeBackingStore::No));
|
||||
raw_data.copy_to(Bytes { backing_store.data, backing_store.size_in_bytes });
|
||||
return AK::adopt_nonnull_ref_or_enomem(new (nothrow) Bitmap(format, alpha_type, size, backing_store));
|
||||
|
|
|
|||
|
|
@ -18,5 +18,6 @@ target_link_libraries(BenchmarkJPEGLoader PRIVATE LibImageDecoders)
|
|||
target_link_libraries(TestImageDecoder PRIVATE LibImageDecoders)
|
||||
target_link_libraries(TestImageWriter PRIVATE LibImageDecoders)
|
||||
|
||||
ladybird_test(TestBitmapSequence.cpp LibGfx LIBS LibGfx LibIPC)
|
||||
ladybird_test(TestShareableBitmap.cpp LibGfx LIBS LibGfx LibIPC)
|
||||
ladybird_test(TestYUVData.cpp LibGfx LIBS LibGfx skia)
|
||||
|
|
|
|||
77
Tests/LibGfx/TestBitmapSequence.cpp
Normal file
77
Tests/LibGfx/TestBitmapSequence.cpp
Normal file
|
|
@ -0,0 +1,77 @@
|
|||
/*
|
||||
* Copyright (c) 2026, Ladybird contributors
|
||||
*
|
||||
* SPDX-License-Identifier: BSD-2-Clause
|
||||
*/
|
||||
|
||||
#include <AK/Array.h>
|
||||
#include <AK/MemoryStream.h>
|
||||
#include <AK/Queue.h>
|
||||
#include <LibCore/AnonymousBuffer.h>
|
||||
#include <LibGfx/Bitmap.h>
|
||||
#include <LibGfx/BitmapSequence.h>
|
||||
#include <LibIPC/Attachment.h>
|
||||
#include <LibIPC/Decoder.h>
|
||||
#include <LibIPC/Encoder.h>
|
||||
#include <LibIPC/Message.h>
|
||||
#include <LibTest/TestCase.h>
|
||||
|
||||
TEST_CASE(ipc_decode_rejects_undersized_single_frame_backing)
|
||||
{
|
||||
// Issue #10036: A single-frame BitmapSequence whose metadata describes a 50000x10 (2,000,000-byte) bitmap but ships
|
||||
// only a 4096-byte backing buffer. Decoding must fail rather than adopt the undersized buffer (and write OOB).
|
||||
constexpr size_t backing_size = 4096;
|
||||
auto buffer = MUST(Core::AnonymousBuffer::create_with_size(backing_size));
|
||||
|
||||
Gfx::BitmapMetadata metadata {
|
||||
.format = Gfx::BitmapFormat::BGRA8888,
|
||||
.alpha_type = Gfx::AlphaType::Premultiplied,
|
||||
.size = Gfx::IntSize { 50000, 10 },
|
||||
.size_in_bytes = backing_size,
|
||||
};
|
||||
|
||||
Vector<Optional<Gfx::BitmapMetadata>> metadata_list;
|
||||
metadata_list.append(metadata);
|
||||
|
||||
// Hand-encode the wire fields in the order IPC::encode(BitmapSequence) uses. The message is internally consistent,
|
||||
// yet inconsistent with the claimed bitmap geometry — which is exactly what the decoder must catch.
|
||||
IPC::MessageBuffer message_buffer;
|
||||
IPC::Encoder encoder { message_buffer };
|
||||
MUST(encoder.encode(metadata_list));
|
||||
MUST(encoder.encode(static_cast<size_t>(backing_size)));
|
||||
MUST(encoder.encode(buffer));
|
||||
|
||||
auto data = message_buffer.take_data();
|
||||
FixedMemoryStream stream { data.span() };
|
||||
|
||||
Queue<IPC::Attachment> attachments;
|
||||
for (auto& attachment : message_buffer.take_attachments())
|
||||
attachments.enqueue(move(attachment));
|
||||
|
||||
IPC::Decoder decoder { stream, attachments };
|
||||
auto result = IPC::decode<Gfx::BitmapSequence>(decoder);
|
||||
EXPECT(result.is_error());
|
||||
}
|
||||
|
||||
TEST_CASE(create_with_anonymous_buffer_rejects_undersized_buffer)
|
||||
{
|
||||
auto buffer = MUST(Core::AnonymousBuffer::create_with_size(4096));
|
||||
auto result = Gfx::Bitmap::create_with_anonymous_buffer(Gfx::BitmapFormat::BGRA8888, Gfx::AlphaType::Premultiplied, move(buffer), Gfx::IntSize { 50000, 10 });
|
||||
EXPECT(result.is_error());
|
||||
}
|
||||
|
||||
TEST_CASE(create_with_anonymous_buffer_accepts_correctly_sized_buffer)
|
||||
{
|
||||
Gfx::IntSize size { 64, 64 };
|
||||
auto required = Gfx::Bitmap::size_in_bytes(Gfx::Bitmap::minimum_pitch(size.width(), Gfx::BitmapFormat::BGRA8888), size.height());
|
||||
auto buffer = MUST(Core::AnonymousBuffer::create_with_size(required));
|
||||
auto bitmap = Gfx::Bitmap::create_with_anonymous_buffer(Gfx::BitmapFormat::BGRA8888, Gfx::AlphaType::Premultiplied, move(buffer), size);
|
||||
EXPECT(!bitmap.is_error());
|
||||
}
|
||||
|
||||
TEST_CASE(create_with_raw_data_rejects_undersized_data)
|
||||
{
|
||||
Array<u8, 16> tiny {};
|
||||
auto result = Gfx::Bitmap::create_with_raw_data(Gfx::BitmapFormat::BGRA8888, Gfx::AlphaType::Premultiplied, tiny, Gfx::IntSize { 50000, 10 });
|
||||
EXPECT(result.is_error());
|
||||
}
|
||||
Loading…
Reference in a new issue