It was possible to go outside the interlacing row strid/offset arrays.
Just fail the decode if this is about to happen. I've added a FIXME
about rejecting such images earlier, since it's a bit sad to only do
this once we realize the pass index is about to overflow.
Found by oss-fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28239
Problem:
- Functions are duplicated in [PBM,PGM,PPM]Loader class
implementations. They are functionally equivalent. This does not
follow the DRY (Don't Repeat Yourself) principle.
Solution:
- Factor out the common functions into a separate file.
- Refactor common code to generic functions.
- Change `PPM_DEBUG` macro to be `PORTABLE_IMAGE_LOADER_DEBUG` to work
with all the supported types. This requires adding the image type to
the debug log messages for easier debugging.
Problem:
- `Streamer` is the same in [PBM,PGM,PPM]Loader class implementations.
Solution:
- Extract it to its own header file to reduce maintenance burden.
- Implement `read` in terms of `read_bytes` to make the class "DRY".
- Decorate all functions with `constexpr`.
Problem:
- `(void)` simply casts the expression to void. This is understood to
indicate that it is ignored, but this is really a compiler trick to
get the compiler to not generate a warning.
Solution:
- Use the `[[maybe_unused]]` attribute to indicate the value is unused.
Note:
- Functions taking a `(void)` argument list have also been changed to
`()` because this is not needed and shows up in the same grep
command.
This patchset fixes:
- Some parts of the path being skipped and not drawn (often horizontal)
- The filled shape moving around on an int grid depending on the winding
number
- Winding number mess-up with four-way intersections
Remove ACCURATE_GAMMA_ADJUSTMENT, since it makes the implementation
uglier, isn't guaranteed to make gamma adjustment accurate and is much
slower. gamma_accurate_blend4 should either be always used or not
exist based on compilation flags, so there is no need to have it in
its own function. Finally, we should use AK/SIMD.h instead of defining
our own f32x4 type.
When create_with_shared_buffer() is called in the next line, the
RefPtr::operator* asserts that the RefPtr is not null. That can happen when
we're low-ish on memory, and the image is huge.
Previously, in the case of RLE4, parsing took suspiciously long.
What happened was that 'pixel_count' was 255, and 'i' was incremented
by *two* in each iteration, so the for-loop continued until the
entire output buffer was full, and then rejected the RLE data
as bogus.
This little diff allows pixel_count to reach 256, be greater than
pixel_count, and thus terminate the loop in the intended way.
- OSv2 DIBs were not checked at all
- Regular Info DIBs had the compression checked after applying a 0xFF mask,
which let many invalid compression values pass.
- There may still be a separate latent bug that causes mask_sizes and mask_shifts to be empty.
This is a hack which can be removed once GitHub Actions changes the
default version to clang 11.
This is apparently sometime in mid-December.
Note, clang-11 is not currently available on Ubuntu 20.04. However,
GitHub Actions uses 20.04, which probably means clang-11 will
become available around that time for all 20.04 users.
The JPEG spec allows component IDs to be chosen arbitrarily from the
interval [0, 255]. Storing components in a vector corrupts the decoder
when component IDs are not in the range 0-3. Normally, encoders don't
use IDs outside of that range because JPEG doesn't support more than
4 channels. But since there is a chance that a spec compliant JPEG
would have component IDs outside of [0-3], we should consider replacing
the vector, which enforces serial component access based on component
IDs, with a HashMap<u8, ComponentSpec>.
Switch over to gamma-aware interpolation. This causes color gradients
to not look so dark in the middle. SIMD optimized code is provided for
sse1 enabled builds.
Fixes#1342.
This doesn't fix all the issues found by the fuzzer, but it fixes
many of them. When running this
Meta/Lagom/Fuzzers/FuzzJPGLoader -jobs=24 -workers=24 \
../Base/res/html/misc/jpgsuite_files/
for 10 minutes on my machine, the fuzzer foudn 2 crashers, but after
this change it finds just ... 2. But with different stacks!
This just fixes ASSERT()s, so it's not security critical, but
ASSERT()s still crash the programs decoding JPGs, and crashing
less is nice even if it's not a security concern.
