diff --git a/src/changes/changes.xml b/src/changes/changes.xml index b38b1a9905..849602c319 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -53,6 +53,7 @@ The type attribute can be add,update,fix,remove. Fix int overflow in PCX image buffer sizing, now throws AllocationRequestException (#552). Fix int overflow in Allocator.array/arrayList size checks (#551). Add messages when throwing NullPointerException. + Fix int overflow in the JPEG raster allocation guard, now throws AllocationRequestException (#570). Add unit tests (#553). diff --git a/src/main/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoder.java b/src/main/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoder.java index 56cea22975..ffd9f1eba2 100644 --- a/src/main/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoder.java +++ b/src/main/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoder.java @@ -443,7 +443,8 @@ public void visitSos(final int marker, final byte[] markerBytes, final byte[] im final int[] preds = Allocator.intArray(sofnSegment.numberOfComponents); final ColorModel colorModel; final WritableRaster raster; - Allocator.check(Integer.BYTES * sofnSegment.width * sofnSegment.height); + // Compute the request in long so a large width times height cannot overflow int and slip past the guard. + Allocator.check((long) sofnSegment.width * sofnSegment.height, Integer.BYTES); switch (sofnSegment.numberOfComponents) { case 4: // Special handling for the application-RGB case: TIFF files with diff --git a/src/test/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoderTest.java b/src/test/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoderTest.java index c4bd42c410..42f392cfab 100644 --- a/src/test/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoderTest.java +++ b/src/test/java/org/apache/commons/imaging/formats/jpeg/decoder/JpegDecoderTest.java @@ -17,12 +17,14 @@ package org.apache.commons.imaging.formats.jpeg.decoder; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.File; import org.apache.commons.imaging.ImagingException; import org.apache.commons.imaging.bytesource.ByteSource; +import org.apache.commons.imaging.common.AllocationRequestException; import org.apache.commons.imaging.test.TestResources; import org.junit.jupiter.api.Test; @@ -41,4 +43,24 @@ void testDecodeBadFile() { final ByteSource byteSourceFile = ByteSource.file(inputFile); assertThrows(ImagingException.class, () -> new JpegDecoder().decode(byteSourceFile)); } + + /** + * A SOF whose width times height, scaled by {@link Integer#BYTES}, overflows a 32-bit int must not slip past the raster allocation guard (IMAGING-379). + * Here 0x8000 by 0x8000 makes {@code Integer.BYTES * width * height} wrap to zero; the request is now computed in long and still rejected. + */ + @Test + void testOversizedRasterAllocationIsRejected() { + final byte[] jpeg = { + (byte) 0xFF, (byte) 0xD8, // SOI + (byte) 0xFF, (byte) 0xC0, 0x00, 0x0B, // SOF0, segment length 11 + 0x08, (byte) 0x80, 0x00, (byte) 0x80, 0x00, // precision 8, height 0x8000, width 0x8000 + 0x01, 0x01, 0x11, 0x00, // one component: id 1, sampling 0x11, quantization table 0 + (byte) 0xFF, (byte) 0xDA, 0x00, 0x08, // SOS, segment length 8 + 0x01, 0x01, 0x00, 0x00, 0x3F, 0x00, // one component: selector 1, tables 0; Ss 0, Se 63, Ah/Al 0 + 0x00, 0x00, // minimal entropy payload + (byte) 0xFF, (byte) 0xD9, // EOI + }; + final ImagingException ex = assertThrows(ImagingException.class, () -> new JpegDecoder().decode(ByteSource.array(jpeg))); + assertInstanceOf(AllocationRequestException.class, ex.getCause()); + } }