Skip to content

Commit

Permalink
LibGfx/JPEG: Remove the ensure_bounds_okay function
Browse files Browse the repository at this point in the history
This function has probably been added when we weren't as good with error
propagations as we are now. We can safely remove it and let future
calls to `read` fail if the file is corrupted.

This can be tested with the following bytes (already used in 9191829):
ffd8ffc000000800080ef701101200ffda00030100
  • Loading branch information
LucasChollet authored and trflynn89 committed Apr 3, 2023
1 parent 402c9e5 commit dc9e783
Showing 1 changed file with 2 additions and 18 deletions.
20 changes: 2 additions & 18 deletions Userland/Libraries/LibGfx/ImageFormats/JPEGLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,15 +554,6 @@ static ErrorOr<void> decode_huffman_stream(JPEGLoadingContext& context, Vector<M
return {};
}

static inline ErrorOr<void> ensure_bounds_okay(const size_t cursor, const size_t delta, const size_t bound)
{
if (Checked<size_t>::addition_would_overflow(delta, cursor))
return Error::from_string_literal("Bounds are not ok: addition would overflow");
if (delta + cursor >= bound)
return Error::from_string_literal("Bounds are not ok");
return {};
}

static bool is_frame_marker(Marker const marker)
{
// B.1.1.3 - Marker assignments
Expand Down Expand Up @@ -633,8 +624,7 @@ static ErrorOr<void> read_start_of_scan(AK::SeekableStream& stream, JPEGLoadingC
return Error::from_string_literal("SOS found before reading a SOF");
}

u16 bytes_to_read = TRY(stream.read_value<BigEndian<u16>>()) - 2;
TRY(ensure_bounds_okay(TRY(stream.tell()), bytes_to_read, TRY(stream.size())));
[[maybe_unused]] u16 const bytes_to_read = TRY(stream.read_value<BigEndian<u16>>()) - 2;
u8 const component_count = TRY(stream.read_value<u8>());

Scan current_scan;
Expand Down Expand Up @@ -711,7 +701,6 @@ static ErrorOr<void> read_restart_interval(AK::SeekableStream& stream, JPEGLoadi
static ErrorOr<void> read_huffman_table(AK::SeekableStream& stream, JPEGLoadingContext& context)
{
i32 bytes_to_read = TRY(stream.read_value<BigEndian<u16>>());
TRY(ensure_bounds_okay(TRY(stream.tell()), bytes_to_read, TRY(stream.size())));
bytes_to_read -= 2;
while (bytes_to_read > 0) {
HuffmanTableSpec table;
Expand Down Expand Up @@ -865,7 +854,6 @@ static ErrorOr<void> read_colour_encoding(SeekableStream& stream, [[maybe_unused
static ErrorOr<void> read_app_marker(SeekableStream& stream, JPEGLoadingContext& context, int app_marker_number)
{
i32 bytes_to_read = TRY(stream.read_value<BigEndian<u16>>());
TRY(ensure_bounds_okay(TRY(stream.tell()), bytes_to_read, TRY(stream.size())));

if (bytes_to_read <= 2)
return Error::from_string_literal("app marker size too small");
Expand Down Expand Up @@ -931,10 +919,7 @@ static ErrorOr<void> read_start_of_frame(AK::SeekableStream& stream, JPEGLoading
return Error::from_string_literal("SOF repeated");
}

i32 bytes_to_read = TRY(stream.read_value<BigEndian<u16>>());

bytes_to_read -= 2;
TRY(ensure_bounds_okay(TRY(stream.tell()), bytes_to_read, TRY(stream.size())));
[[maybe_unused]] u16 const bytes_to_read = TRY(stream.read_value<BigEndian<u16>>());

context.frame.precision = TRY(stream.read_value<u8>());
if (context.frame.precision != 8) {
Expand Down Expand Up @@ -1006,7 +991,6 @@ static ErrorOr<void> read_start_of_frame(AK::SeekableStream& stream, JPEGLoading
static ErrorOr<void> read_quantization_table(AK::SeekableStream& stream, JPEGLoadingContext& context)
{
i32 bytes_to_read = TRY(stream.read_value<BigEndian<u16>>()) - 2;
TRY(ensure_bounds_okay(TRY(stream.tell()), bytes_to_read, TRY(stream.size())));
while (bytes_to_read > 0) {
u8 info_byte = TRY(stream.read_value<u8>());
u8 element_unit_hint = info_byte >> 4;
Expand Down

0 comments on commit dc9e783

Please sign in to comment.