Some WebP files advertise ICC metadata in their VP8X feature flags even
though the ICCP chunk is absent or malformed. We accepted the bitstream
header and could decode the pixels, but the follow-up mux metadata
lookup failed during header decoding and made sniffing reject the image
entirely.
Treat mux and ICC metadata extraction as best-effort after libwebp
accepts the header. Images without readable optional ICC metadata now
decode normally and simply report no ICC profile.
Problem: A BMP V5 image whose embedded ICC profile offset points past
the end of the file triggers an OOB read.
Cause: The bounds check summed the profile offset, the file-header size,
and the profile size in 32-bit arithmetic. So, a large offset (e.g.
0xfffffff0) wraps the sum back into range and passes the check. The
decoder then returns a span pointing far past the end of the file.
Fix: Compute the sum in 64 bits — so an out-of-bounds offset can no
longer wrap, and the profile’s rejected.
Fixes: https://github.com/LadybirdBrowser/ladybird/issues/9967
Problem: Decoding a BMP whose height is INT_MIN triggered a UBSan error.
A top-down BMP legitimately uses a negative height — and unlike width,
it’s not rejected, so it can be INT_MIN.
Cause: decode_bmp_pixel_data() took the magnitude of the i32 width and
height with abs(). Negating INT_MIN is undefined behavior.
Fix: Widen to i64 before taking the absolute value — so the magnitude of
INT_MIN is representable. The resulting out-of-range dimension is still
rejected by Bitmap::create — so only the undefined behavior changes.
Fixes: https://github.com/LadybirdBrowser/ladybird/issues/9994
Problem: Decoding a non-square image whose Exif orientation is 5
(transpose) writes one pixel past the end of the destination bitmap —
an ASan heap-buffer-overflow in ExifOrientedBitmap::set_pixel.
Cause: oriented_position() mapped orientation 5 by composing the
“flip-horizontally” and “rotate-90-clockwise” helpers. Each helper
mirrors using the source width. But after the rotate, the point is
already in the transposed coordinate space — where the relevant
dimension is the source height. For a non-square image, that composition
produces x-coordinates past the destination width. Only square images
happened to stay in bounds. The destination bitmap is the transposed
size — so the out-of-range column wrote past its allocation.
Fix: Map orientation 5 directly as a transpose across the main diagonal:
source (x, y) to destination (y, x).
Fixes https://github.com/LadybirdBrowser/ladybird/issues/10102
Replace the homegrown GIF parser and LZW decompressor with the wuffs
GIF decoder, which is memory-safe by construction and already used in
other engines via Skia.
One behavior change is that `loop_count()` now reports the correct
value, since the raw value stored in the file does not include the
first frane and should be incremented by 1 to be compatible with what
callers expect.
Problem: Decoding a BI_RLE24 BMP binds a u32 reference to a misaligned
address, gets flagged by UBSan while decompressing the run-length data.
Cause: The decompressed RLE24 buffer holds 24-bit pixels at a 3-byte
stride (decode_bmp_pixel_data reads it back with LE read_u24) — but each
pixel was getting stored with a 4-byte write at 3-byte-strided offsets.
Fix: Store the 24-bit value as three LE bytes — so the write
matches the stride and is always aligned. Size the buffer to the
real 3-bytes-per-pixel total, and bound-check the 3-byte write.
Fixes https://github.com/LadybirdBrowser/ladybird/issues/9958
Remove the TinyVG decoder now that the Qt chrome no longer depends on
TVG resources. Drop the decoder registration, MIME and supported image
type entries, fuzzer target, decoder tests, and TinyVG test inputs.
When choosing the best image from an ICO file, prefer the entry with the
largest pixel area first, and only use bits-per-pixel as a tie-breaker.
This had regressed in commit b10fe7c136.
This fixes ICO files like Discord's favicon, where 16x16, 32x32, 48x48,
and 256x256 entries all advertise the same bpp. We were previously just
choosing whichever we saw first, which happened to be the 16x16 icon.
Problem: Runs of the Linux Sanitizers intermittently fail with
LeakSanitizer leaks of ~45 KB across 21 allocations from
PNGLoadingContext::read_frames() — triggered when the test suite
decodes malformed PNGs.
Cause: When libpng hits a corrupted IDAT chunk inside png_read_image(),
it longjmps back to the setjmp landing pad in
PNGLoadingContext::read_all_frames(). longjmp unwinds the stack without
running C++ destructors — so the stack-locals in read_frames
(Vector<u8*> row_pointers, the in-flight Bitmap inside decode_frame, and
the APNG branch’s output buffer and Painter) leak their heap storage.
Fix: Promote those stack-locals to members of PNGLoadingContext (which
is heap-allocated and outlives the setjmp scope) — so their storage is
reachable by RAII when the context is destroyed. Clear them in the
setjmp error handler too — so memory is released promptly on the error
path, rather than waiting until ~PNGLoadingContext().
DecodedImageFrame only wraps a ref-counted Bitmap and color-space
metadata. The frame object itself does not provide shared mutable
state or lifetime ownership beyond those members, so ref-counting it
adds an unnecessary layer of indirection.
Decoded image data should not continue to traffic in ImmutableBitmap now
that the bitmap wrapper is being retired. Introduce DecodedImageFrame as
the paintable decoded-image unit and store a Bitmap plus ColorSpace in
it directly.
Thread the new frame type through decoded image data, display-list
image commands, filters, canvas drawImage, patterns, WebGL texture
upload, and CSS/SVG image consumers. ImmutableBitmap remains only at
the legacy boundaries that still need it, such as HTML video snapshots
and callers that explicitly ask for a bitmap snapshot.
This keeps color-space ownership with the decoded frame while making
the expensive or legacy ImmutableBitmap path explicit at the few call
sites that still need it.
ImageDecoder itself can check the matrix coefficients and range flag
instead. In the future, ImageDecoder should probably convert its images
into RGB color space using the same solution as for video.
libavif outputs straight (unpremultiplied) alpha by default, but the
AVIF loader was creating bitmaps without specifying an alpha type,
which defaults to premultiplied. This mismatch caused semi-transparent
pixels to render incorrectly.
Pass AlphaType::Unpremultiplied explicitly, matching what the PNG,
WebP, and BMP decoders already do.
Instead of decoding all animation frames at once in decode_webp_image()
and caching them in frame_descriptors, decode frames one at a time
on demand. This avoids holding all decoded frames in memory at once.
The WebPAnimDecoder is kept alive across frame() calls and supports
reset for backward seeks (needed for animation looping).
Add a virtual method to query frame durations without decoding pixel
data. This is needed by the ImageDecoder service to extract timing
metadata upfront for streaming animation decode.
Implement the method for GIF, PNG, WebP, AVIF, and JPEGXL decoders.
For WebP, extract durations from the demuxer during header decode so
they are available before any frames are decoded.
The `Bitmap` type was referring to to its internal pixel format by a
name that represents the order of the color components as they are layed
out in memory. Contrary, the `Color` type was using a naming that where
the name represents the order of the components from most to least
significant byte when viewed as a unsigned 32bit integer. This is
confusing as you have to keep remembering which mental model to use
depending on which code you work with.
To unify the two, the naming of RGBA-like colors in the `Color` type has
been adjusted to match the one from the Bitmap type. This seems to be
generally in line with how web APIs think about these types:
* `ImageData.pixelFormat` can be `rgba-8unorm` backed by a
`Uint8ClamedArray`, but there is no pixel format backed by a 32bit
unsigned type.
* WebGL can use format `RGBA` with type `UNSIGNED_BYTE`, but there is no
such format with type `UNSIGNED_INT`.
Additionally, it appears that other browsers and browser-adjacent
libraries also think similarly about these types:
* Firefox:
https://github.com/mozilla-firefox/firefox/blob/main/gfx/2d/Types.h
* WebKit:
https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/graphics/PixelFormat.h
* Skia:
https://chromium.googlesource.com/skia/+/refs/heads/main/include/core/SkColorType.h
This has the not so nice side effect that APIs that interact with these
types through 32bit unsigned integers now have the component order
inverted due to little-endian byte order. E.g. specifying a color as hex
constant needs to be done as `0xAABBGGRR` if it is to be treated as
RGBA8888.
We could alleviate this by providing endian-independent APIs to callers.
But I suspect long-term we might want to think differently about bitmap
data anyway, e.g. to better support HDR in the future. However, such
changes would be more involved than just unifying the naming as done
here. So I considered that out of scope for now.
From what I can tell BMP files with an alpha channel always store
unpremultiplied alpha. So let's load them as such to avoid rendering
artifacts from using the wrong alpha type.
We should invert CMYK data only if color space is JCS_CMYK and either
there is no Adobe marker, or the Adobe transform is 0. Transform 2
indicates YCCK data, which we should not invert.
The decoder was requiring GIF files to be at least 32 bytes, but the
actual minimum for a valid GIF is only 26 bytes:
- 6 bytes for the header
- 7 bytes for the Logical Screen Descriptor
- 10 bytes for the Image Descriptor
- 2 bytes for the LZW minimum code size and block terminator
- 1 byte for the GIF trailer
This change allows us to load minimal 1x1 GIFs with empty LZW data.
They are commonly used on the web as transparent placeholders with
minimal file size.
As it turns out, SkPath already behaves the way we need for SVG and HTML
canvas elements. Less work for us, yay! This removes a 5% item from the
profile when scrolling on https://imdb.com/
Note that there's a tiny screenshot test expectation change due to
minor antialiasing differences when we no longer do our redundant
subpath modifications.
Gfx::Bitmap only supports a bit depth of 8, therefore we refused to
load AVIF images which didn't have this bit depth.
However, we can tell the libavif decoder to reduce the output depth by
setting avifRGBImage.depth to 8. This allows us to support any input
depth.
Makes images load on https://www.ikea.com/ which uses Cloudflare Images
to re-encode their images to 16-bit AVIF.
In the previous fix, we were still drawing IDAT data to the reference
frame even when no fcTL was present. This would cause rendering issues
when subsequent frames use APNG_BLEND_OP_OVER blending mode, as they
would composite over the incorrect reference frame. This commit adds a
simple check to properly skip any frame without an fcTL chunk.
We now properly handle OS/2 format BMPs that use 3 bytes per color
entry instead of 4. While OS/2 2.x officially specified 4 bytes per
color, some tools still produce files with 3-byte entries. We can
identify such files by checking the available color table space.
This extends ICO loader to support Windows cursor files. There is no
point in creating a separate loader for this, as the ICO format is
very similar to the CUR format. The only differences are bytes used to
identify the file and a presence of a hotspot in the CUR header.
Color masks should only be used when the compression type is either
BITFIELDS or ALPHABITFIELDS. They were always read before and produced
corrupted images when there was random data in the mask fields.
Other browsers don't think that BMP files with more than 1024 colors are
invalid. They clamp the palette instead, and now we do the same. This
allows us to load more BMPs.
Before this change, IDAT data was mistakenly always included in the
animation. Now we only include frames with explicit fcTL chunks.
As per the PNG spec (third edition):
"The static image may be included as the first frame of the animation
by the presence of a single fcTL chunk before IDAT. Otherwise, the
static image is not part of the animation."
We also fall back to the IDAT data when APNG has acTL but no fcTL
chunks. Test image is 062.png from fDAT-inherits-cICP.html from WPT.
GIF loader was completely failing when encountering errors with
frame descriptors or individual frames, even when some frames were
successfully loaded. Now we attempt to decode at least some frames
and fail only when no frames can be decoded at all.
This introduces a new API in ImageDecoderPlugins that allow an image
decoder to return a CICP struct. Also, we use this API in
ImageDecoder::color_space() to create a color space corresponding to
these CICP.
