mirror of
https://github.com/libjxl/libjxl.git
synced 2026-10-28 10:15:22 +08:00
Reject PNM/PAM samples above MaxVal (#5064)
Netpbm only allows sample values in [0, MaxVal], but the PNM reader only checked MaxVal itself and then copied the raster as-is. It also sets JXL_BIT_DEPTH_FROM_CODESTREAM, whose documented input range is 0..(1 << bits_per_sample) - 1, and nothing downstream enforces that range. So a P6 with MaxVal 4095 and a sample of 65535 was encoded with bits_per_sample = 12 and modular_16_bit_buffer_sufficient = true, while the codestream holds values that do not fit in 16-bit signed buffers. libjxl's decoder uses int32 buffers and clamps on output, so djxl shows 4095. Decoders that use 16-bit buffers when the header says they are sufficient (jxl-rs, used by Chrome) wrap samples above 32767 to negative values, which render as black. Check the samples in DecodeImagePNM and in ChunkedPNMDecoder::Init (cjxl --streaming_input) and fail if any is above MaxVal. MaxVal is always 2^n - 1 here, so a sample is too large iff its most significant byte has a bit set above MaxVal; the check ORs the raster 8 bytes at a time and tests the bytes that hold those bits. It is skipped for MaxVal 255 and 65535, where every value is valid, and for PFM. On a 6000x4000 12-bit PPM, cjxl -d 0 -e 1 goes from about 0.10 s to 0.11 s. Tested: - New CodecTest.PNMSampleAboveMaxVal: P5/P6/P7 inputs with samples equal to MaxVal decode; a sample above MaxVal (4/12-bit, colour and alpha, at every position of a 19-sample row) fails. It fails without the pnm.cc change. - ctest (full suite). - cjxl -d 0, -d 1 and -d 0 --streaming_input on random PGM/PPM/PAM inputs with MaxVal 1, 15, 255, 1023, 4095, 16383 and 65535 (plus all-MaxVal images) give byte-identical output to the unpatched cjxl; inputs with a sample above MaxVal are now rejected. Co-authored-by: Eugene Kliuchnikov <eustas@google.com>
This commit is contained in:
1 parent
ee558e9540
commit
ec2982b1f7
3 files changed
+80
No files matched your search
@@ -82,6 +82,7 @@ L. E. Segovia
|
||||
ledoge
|
||||
Leo Izen <leo.izen@gmail.com>
|
||||
Linus VandeVondele <linus.vandevondele@gmail.com>
|
||||
LongYinan <lynweklm@gmail.com>
|
||||
Lovell Fuller
|
||||
Lucas Chollet <lucas.chollet@serenityos.org>
|
||||
Maarten DB <anonymous.maarten@gmail.com>
|
||||
|
||||
@@ -435,6 +435,49 @@ TEST(CodecTest, TestPNM) {
|
||||
ASSERT_TRUE(std::abs(d - -3.141592) < 1E-15);
|
||||
}
|
||||
|
||||
TEST(CodecTest, PNMSampleAboveMaxVal) {
|
||||
const auto decode = [](const std::string& header,
|
||||
const std::vector<uint8_t>& samples) -> bool {
|
||||
std::vector<uint8_t> pnm(header.begin(), header.end());
|
||||
pnm.insert(pnm.end(), samples.begin(), samples.end());
|
||||
PackedPixelFile ppf;
|
||||
return static_cast<bool>(DecodeBytes(Bytes(pnm), ColorHints(), &ppf));
|
||||
};
|
||||
const std::string kPam12 =
|
||||
"P7\nWIDTH 1\nHEIGHT 1\nDEPTH 4\nMAXVAL 4095\n"
|
||||
"TUPLTYPE RGB_ALPHA\nENDHDR\n";
|
||||
|
||||
// Samples up to MaxVal are valid.
|
||||
EXPECT_TRUE(decode("P5\n2 1\n15\n", {0, 15}));
|
||||
EXPECT_TRUE(decode("P5\n2 1\n255\n", {0, 255}));
|
||||
EXPECT_TRUE(decode("P6\n1 1\n4095\n", {0x0F, 0xFF, 0, 0, 0x0F, 0xFF}));
|
||||
EXPECT_TRUE(decode("P5\n1 1\n65535\n", {0xFF, 0xFF}));
|
||||
EXPECT_TRUE(decode(kPam12, {0, 0, 0, 0, 0, 0, 0x0F, 0xFF}));
|
||||
|
||||
#if (!JXL_CRASH_ON_ERROR)
|
||||
// Samples above MaxVal are rejected.
|
||||
EXPECT_FALSE(decode("P5\n2 1\n15\n", {0, 16}));
|
||||
EXPECT_FALSE(decode("P6\n1 1\n4095\n", {0x0F, 0xFF, 0x10, 0x00, 0, 0}));
|
||||
EXPECT_FALSE(decode("P6\n1 1\n4095\n", {0, 0, 0, 0, 0xFF, 0xFF}));
|
||||
EXPECT_FALSE(decode(kPam12, {0, 0, 0, 0, 0, 0, 0x10, 0x00}));
|
||||
// Check every sample position of images larger than 8 bytes.
|
||||
const std::vector<uint8_t> max4(19, 15);
|
||||
std::vector<uint8_t> max12;
|
||||
for (size_t i = 0; i < 19; ++i) max12.insert(max12.end(), {0x0F, 0xFF});
|
||||
EXPECT_TRUE(decode("P5\n19 1\n15\n", max4));
|
||||
EXPECT_TRUE(decode("P5\n19 1\n4095\n", max12));
|
||||
for (size_t i = 0; i < 19; ++i) {
|
||||
std::vector<uint8_t> samples4 = max4;
|
||||
samples4[i] = 16;
|
||||
EXPECT_FALSE(decode("P5\n19 1\n15\n", samples4)) << i;
|
||||
std::vector<uint8_t> samples12 = max12;
|
||||
samples12[2 * i] = 0x10;
|
||||
samples12[2 * i + 1] = 0x00;
|
||||
EXPECT_FALSE(decode("P5\n19 1\n4095\n", samples12)) << i;
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
TEST(CodecTest, FormatNegotiation) {
|
||||
const std::vector<JxlPixelFormat> accepted_formats = {
|
||||
{/*num_channels=*/4,
|
||||
|
||||
@@ -340,6 +340,35 @@ class Parser {
|
||||
const uint8_t* const end_;
|
||||
};
|
||||
|
||||
// Netpbm requires every sample to be in [0, MaxVal]. MaxVal is 2^bits - 1, so
|
||||
// a sample is out of range iff it has a bit set at or above `bits_per_sample`.
|
||||
// Samples are big-endian, so for 16-bit storage only the first byte of each
|
||||
// sample can hold such bits. `data` holds `size` bytes of integer samples.
|
||||
Status VerifySampleRange(const uint8_t* data, size_t size,
|
||||
size_t bits_per_sample) {
|
||||
// Every value of the storage type is a valid sample.
|
||||
if (bits_per_sample == 8 || bits_per_sample == 16) return true;
|
||||
// OR the input 8 bytes at a time: byte k of `acc` is then the OR of all
|
||||
// bytes at offsets equal to k modulo 8.
|
||||
uint64_t acc = 0;
|
||||
size_t i = 0;
|
||||
for (; i + 8 <= size; i += 8) {
|
||||
uint64_t v;
|
||||
memcpy(&v, data + i, 8);
|
||||
acc |= v;
|
||||
}
|
||||
uint8_t acc_bytes[8];
|
||||
memcpy(acc_bytes, &acc, 8);
|
||||
for (; i < size; ++i) acc_bytes[i % 8] |= data[i];
|
||||
const size_t step = bits_per_sample > 8 ? 2 : 1;
|
||||
const size_t shift =
|
||||
bits_per_sample > 8 ? bits_per_sample - 8 : bits_per_sample;
|
||||
uint8_t bits = 0;
|
||||
for (size_t k = 0; k < 8; k += step) bits |= acc_bytes[k];
|
||||
if (bits >> shift) return JXL_FAILURE("PNM: sample value exceeds MaxVal");
|
||||
return true;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
struct PNMChunkedInputFrame {
|
||||
@@ -432,6 +461,9 @@ StatusOr<ChunkedPNMDecoder> ChunkedPNMDecoder::Init(
|
||||
if (size < required_size) {
|
||||
return JXL_FAILURE("PNM file too small");
|
||||
}
|
||||
JXL_RETURN_IF_ERROR(VerifySampleRange(dec.pnm_.data() + dec.data_start_,
|
||||
required_size - dec.data_start_,
|
||||
header.bits_per_sample));
|
||||
return dec;
|
||||
}
|
||||
|
||||
@@ -555,6 +587,10 @@ Status DecodeImagePNM(const Span<const uint8_t> bytes,
|
||||
if (pnm_remaining_size < required_pnm_size) {
|
||||
return JXL_FAILURE("PNM file too small");
|
||||
}
|
||||
if (!header.floating_point) {
|
||||
JXL_RETURN_IF_ERROR(
|
||||
VerifySampleRange(pos, required_pnm_size, header.bits_per_sample));
|
||||
}
|
||||
|
||||
ppf->frames.clear();
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user