diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 201c655091..39c25be421 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -51,6 +51,7 @@ The type attribute can be add,update,fix,remove. Fix Apache RAT plugin console warnings. Fix Javadoc warnings. Fix int overflow in PCX image buffer sizing, now throws AllocationRequestException (#552). + Fix int overflow in ImageBuilder pixel array sizing, now throws AllocationRequestException. Fix int overflow in Allocator.array/arrayList size checks (#551). Add messages when throwing NullPointerException. diff --git a/src/main/java/org/apache/commons/imaging/common/Allocator.java b/src/main/java/org/apache/commons/imaging/common/Allocator.java index c8c885760b..d3c39b82b9 100644 --- a/src/main/java/org/apache/commons/imaging/common/Allocator.java +++ b/src/main/java/org/apache/commons/imaging/common/Allocator.java @@ -222,6 +222,18 @@ public static int[] intArray(final int request) { return new int[check(request, Integer.BYTES)]; } + /** + * Allocates an int array of the requested size. + * + * @param request The requested size is cast down to an int. + * @return A new int array. + * @throws AllocationRequestException Thrown when the request does not fit an int or exceeds the limit. + * @see #check(long, int) + */ + public static int[] intArray(final long request) { + return new int[check(request, Integer.BYTES)]; + } + /** * Allocates a long array of the requested size. * diff --git a/src/main/java/org/apache/commons/imaging/common/ImageBuilder.java b/src/main/java/org/apache/commons/imaging/common/ImageBuilder.java index 2569d3efd2..305ee269dc 100644 --- a/src/main/java/org/apache/commons/imaging/common/ImageBuilder.java +++ b/src/main/java/org/apache/commons/imaging/common/ImageBuilder.java @@ -82,10 +82,11 @@ public ImageBuilder(final int width, final int height, final boolean hasAlpha) { * the ImageBuilder or resulting BufferedImage. * @param isAlphaPremultiplied indicates whether alpha values are pre-multiplied; this setting is relevant only if alpha is true. * @throws RasterFormatException if {@code width} or {@code height} are equal or less than zero + * @throws AllocationRequestException if the number of pixels does not fit an {@code int} or exceeds the allocation limit */ public ImageBuilder(final int width, final int height, final boolean hasAlpha, final boolean isAlphaPremultiplied) { checkDimensions(width, height); - data = Allocator.intArray(width * height); + data = Allocator.intArray((long) width * height); this.width = width; this.height = height; this.hasAlpha = hasAlpha; diff --git a/src/test/java/org/apache/commons/imaging/common/AllocatorTest.java b/src/test/java/org/apache/commons/imaging/common/AllocatorTest.java index 1cf283f83c..ff72e2a914 100644 --- a/src/test/java/org/apache/commons/imaging/common/AllocatorTest.java +++ b/src/test/java/org/apache/commons/imaging/common/AllocatorTest.java @@ -17,6 +17,7 @@ package org.apache.commons.imaging.common; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import org.junit.jupiter.api.Test; @@ -29,6 +30,18 @@ void testArrayListOverflowIsRejected() { assertThrows(AllocationRequestException.class, () -> Allocator.arrayList(536870912)); } + /** 2359300L * 65538 does not fit an int. */ + @Test + void testIntArrayLongRequestOverflowIsRejected() { + assertThrows(AllocationRequestException.class, () -> Allocator.intArray(2359300L * 65538)); + } + + /** A long request that fits an int is allocated as is. */ + @Test + void testIntArrayLongRequest() { + assertEquals(3, Allocator.intArray(3L).length); + } + /** 107374183 * 40 overflows int to 24, slipping past the byte-cost limit. */ @Test void testArrayOverflowIsRejected() { diff --git a/src/test/java/org/apache/commons/imaging/common/ImageBuilderTest.java b/src/test/java/org/apache/commons/imaging/common/ImageBuilderTest.java index 64c5e4b430..b75c7ad42f 100644 --- a/src/test/java/org/apache/commons/imaging/common/ImageBuilderTest.java +++ b/src/test/java/org/apache/commons/imaging/common/ImageBuilderTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -88,6 +89,15 @@ void testConstructorBounds() { /** * Test whether color model is properly applied to buffered images */ + /** + * 2359300 * 65538 does not fit an int and wraps to 4,980,744, which used to be accepted and allocate a 19 MB array for an image that has 154 billion + * pixels. For https://issues.apache.org/jira/browse/IMAGING-385. + */ + @Test + void testConstructorDimensionsOverflowingInt() { + assertThrows(AllocationRequestException.class, () -> new ImageBuilder(2359300, 65538, false)); + } + @Test void testImageColorModel() { ImageBuilder imageBuilder; diff --git a/src/test/java/org/apache/commons/imaging/formats/bmp/BmpImageParserTest.java b/src/test/java/org/apache/commons/imaging/formats/bmp/BmpImageParserTest.java index 9512973800..e442415798 100644 --- a/src/test/java/org/apache/commons/imaging/formats/bmp/BmpImageParserTest.java +++ b/src/test/java/org/apache/commons/imaging/formats/bmp/BmpImageParserTest.java @@ -23,7 +23,9 @@ import java.io.IOException; import org.apache.commons.imaging.ImageInfo; +import org.apache.commons.imaging.Imaging; import org.apache.commons.imaging.ImagingException; +import org.apache.commons.imaging.common.AllocationRequestException; import org.apache.commons.imaging.test.TestResources; import org.junit.jupiter.api.Test; @@ -45,6 +47,20 @@ void testImageForNegativeArraySizeException() throws ImagingException, IOExcepti assertThrows(IllegalArgumentException.class, () -> parser.getImageInfo(bmp, new BmpImagingParameters())); } + /** + * For https://issues.apache.org/jira/browse/IMAGING-385. + *

+ * The header declares a 2359300 x 65538 RLE4 bitmap. The pixel count does not fit an {@code int}; it used to wrap to 4,980,744, slip past the allocation + * limit and then fail with a JDK {@link IllegalArgumentException} when the {@code BufferedImage} was created. It is now rejected with an + * {@link AllocationRequestException}. + *

+ */ + @Test + void testDimensionsOverflowingInt() { + final File bmp = TestResources.resourceToFile("/images/bmp/IMAGING-385/crash-e506da5ae78242d1f3308a35980e95123dde479c.bmp"); + assertThrows(AllocationRequestException.class, () -> Imaging.getBufferedImage(bmp)); + } + /** * For https://issues.apache.org/jira/browse/IMAGING-264. * diff --git a/src/test/resources/images/bmp/IMAGING-385/crash-e506da5ae78242d1f3308a35980e95123dde479c.bmp b/src/test/resources/images/bmp/IMAGING-385/crash-e506da5ae78242d1f3308a35980e95123dde479c.bmp new file mode 100644 index 0000000000..223da6a42d Binary files /dev/null and b/src/test/resources/images/bmp/IMAGING-385/crash-e506da5ae78242d1f3308a35980e95123dde479c.bmp differ