From e26ca0382c8a1c8b1f09cca0be3428718bb64743 Mon Sep 17 00:00:00 2001 From: bae Date: Fri, 16 Nov 2012 11:05:43 +0400 Subject: [PATCH] 8001972: Improve image processing Reviewed-by: prr, ahgross --- .../sun/awt/image/ByteComponentRaster.java | 70 ++++++++++----- .../sun/awt/image/ByteInterleavedRaster.java | 29 +----- .../sun/awt/image/ShortComponentRaster.java | 69 +++++++++----- .../sun/awt/image/ShortInterleavedRaster.java | 29 +----- .../native/sun/awt/image/awt_parseImage.c | 90 +++++++++++++++++-- .../native/sun/awt/medialib/safe_alloc.h | 5 ++ 6 files changed, 191 insertions(+), 101 deletions(-) diff --git a/src/share/classes/sun/awt/image/ByteComponentRaster.java b/src/share/classes/sun/awt/image/ByteComponentRaster.java index c92f76659..49c642e37 100644 --- a/src/share/classes/sun/awt/image/ByteComponentRaster.java +++ b/src/share/classes/sun/awt/image/ByteComponentRaster.java @@ -198,7 +198,7 @@ public class ByteComponentRaster extends SunWritableRaster { } this.bandOffset = this.dataOffsets[0]; - verify(false); + verify(); } /** @@ -857,38 +857,68 @@ public class ByteComponentRaster extends SunWritableRaster { } /** - * Verify that the layout parameters are consistent with - * the data. If strictCheck - * is false, this method will check for ArrayIndexOutOfBounds conditions. If - * strictCheck is true, this method will check for additional error - * conditions such as line wraparound (width of a line greater than - * the scanline stride). - * @return String Error string, if the layout is incompatible with - * the data. Otherwise returns null. + * Verify that the layout parameters are consistent with the data. + * + * The method verifies whether scanline stride and pixel stride do not + * cause an integer overflow during calculation of a position of the pixel + * in data buffer. It also verifies whether the data buffer has enough data + * to correspond the raster layout attributes. + * + * @throws RasterFormatException if an integer overflow is detected, + * or if data buffer has not enough capacity. */ - private void verify (boolean strictCheck) { - // Make sure data for Raster is in a legal range - for (int i=0; i < dataOffsets.length; i++) { + protected final void verify() { + for (int i = 0; i < dataOffsets.length; i++) { if (dataOffsets[i] < 0) { - throw new RasterFormatException("Data offsets for band "+i+ - "("+dataOffsets[i]+ - ") must be >= 0"); + throw new RasterFormatException("Data offsets for band " + i + + "(" + dataOffsets[i] + + ") must be >= 0"); } } int maxSize = 0; int size; - for (int i=0; i < numDataElements; i++) { - size = (height-1)*scanlineStride + (width-1)*pixelStride + - dataOffsets[i]; + // we can be sure that width and height are greater than 0 + if (scanlineStride < 0 || + scanlineStride > (Integer.MAX_VALUE / height)) + { + // integer overflow + throw new RasterFormatException("Incorrect scanline stride: " + + scanlineStride); + } + int lastScanOffset = (height - 1) * scanlineStride; + + if (pixelStride < 0 || + pixelStride > (Integer.MAX_VALUE / width)) + { + // integer overflow + throw new RasterFormatException("Incorrect pixel stride: " + + pixelStride); + } + int lastPixelOffset = (width - 1) * pixelStride; + + if (lastPixelOffset > (Integer.MAX_VALUE - lastScanOffset)) { + // integer overflow + throw new RasterFormatException("Incorrect raster attributes"); + } + lastPixelOffset += lastScanOffset; + + for (int i = 0; i < numDataElements; i++) { + size = lastPixelOffset + dataOffsets[i]; + if (dataOffsets[i] > (Integer.MAX_VALUE - lastPixelOffset)) { + throw new RasterFormatException("Incorrect band offset: " + + dataOffsets[i]); + + } + if (size > maxSize) { maxSize = size; } } if (data.length < maxSize) { - throw new RasterFormatException("Data array too small (should be "+ - maxSize+" )"); + throw new RasterFormatException("Data array too small (should be " + + maxSize + " )"); } } diff --git a/src/share/classes/sun/awt/image/ByteInterleavedRaster.java b/src/share/classes/sun/awt/image/ByteInterleavedRaster.java index b721d6868..4279ce1f9 100644 --- a/src/share/classes/sun/awt/image/ByteInterleavedRaster.java +++ b/src/share/classes/sun/awt/image/ByteInterleavedRaster.java @@ -250,7 +250,7 @@ public class ByteInterleavedRaster extends ByteComponentRaster { } } - verify(false); + verify(); } /** @@ -1292,33 +1292,6 @@ public class ByteInterleavedRaster extends ByteComponentRaster { return createCompatibleWritableRaster(width,height); } - /** - * Verify that the layout parameters are consistent with - * the data. If strictCheck - * is false, this method will check for ArrayIndexOutOfBounds conditions. If - * strictCheck is true, this method will check for additional error - * conditions such as line wraparound (width of a line greater than - * the scanline stride). - * @return String Error string, if the layout is incompatible with - * the data. Otherwise returns null. - */ - private void verify (boolean strictCheck) { - int maxSize = 0; - int size; - - for (int i=0; i < numDataElements; i++) { - size = (height-1)*scanlineStride + (width-1)*pixelStride + - dataOffsets[i]; - if (size > maxSize) { - maxSize = size; - } - } - if (data.length < maxSize) { - throw new RasterFormatException("Data array too small (should be "+ - maxSize+" )"); - } - } - public String toString() { return new String ("ByteInterleavedRaster: width = "+width+" height = " + height diff --git a/src/share/classes/sun/awt/image/ShortComponentRaster.java b/src/share/classes/sun/awt/image/ShortComponentRaster.java index bc39bd2f0..df2c0f7d6 100644 --- a/src/share/classes/sun/awt/image/ShortComponentRaster.java +++ b/src/share/classes/sun/awt/image/ShortComponentRaster.java @@ -198,7 +198,7 @@ public class ShortComponentRaster extends SunWritableRaster { } this.bandOffset = this.dataOffsets[0]; - verify(false); + verify(); } /** @@ -791,38 +791,67 @@ public class ShortComponentRaster extends SunWritableRaster { } /** - * Verify that the layout parameters are consistent with - * the data. If strictCheck - * is false, this method will check for ArrayIndexOutOfBounds conditions. If - * strictCheck is true, this method will check for additional error - * conditions such as line wraparound (width of a line greater than - * the scanline stride). - * @return String Error string, if the layout is incompatible with - * the data. Otherwise returns null. + * Verify that the layout parameters are consistent with the data. + * + * The method verifies whether scanline stride and pixel stride do not + * cause an integer overflow during calculation of a position of the pixel + * in data buffer. It also verifies whether the data buffer has enough data + * to correspond the raster layout attributes. + * + * @throws RasterFormatException if an integer overflow is detected, + * or if data buffer has not enough capacity. */ - private void verify (boolean strictCheck) { - // Make sure data for Raster is in a legal range - for (int i=0; i < dataOffsets.length; i++) { + protected final void verify() { + for (int i = 0; i < dataOffsets.length; i++) { if (dataOffsets[i] < 0) { - throw new RasterFormatException("Data offsets for band "+i+ - "("+dataOffsets[i]+ - ") must be >= 0"); + throw new RasterFormatException("Data offsets for band " + i + + "(" + dataOffsets[i] + + ") must be >= 0"); } } int maxSize = 0; int size; - for (int i=0; i < numDataElements; i++) { - size = (height-1)*scanlineStride + (width-1)*pixelStride + - dataOffsets[i]; + // we can be sure that width and height are greater than 0 + if (scanlineStride < 0 || + scanlineStride > (Integer.MAX_VALUE / height)) + { + // integer overflow + throw new RasterFormatException("Incorrect scanline stride: " + + scanlineStride); + } + int lastScanOffset = (height - 1) * scanlineStride; + + if (pixelStride < 0 || + pixelStride > (Integer.MAX_VALUE / width)) + { + // integer overflow + throw new RasterFormatException("Incorrect pixel stride: " + + pixelStride); + } + int lastPixelOffset = (width - 1) * pixelStride; + + if (lastPixelOffset > (Integer.MAX_VALUE - lastScanOffset)) { + // integer overflow + throw new RasterFormatException("Incorrect raster attributes"); + } + lastPixelOffset += lastScanOffset; + + for (int i = 0; i < numDataElements; i++) { + size = lastPixelOffset + dataOffsets[i]; + if (dataOffsets[i] > (Integer.MAX_VALUE - lastPixelOffset)) { + throw new RasterFormatException("Incorrect band offset: " + + dataOffsets[i]); + } + if (size > maxSize) { maxSize = size; } } if (data.length < maxSize) { - throw new RasterFormatException("Data array too small (should be "+ - maxSize+" )"); + throw new RasterFormatException("Data array too small (should be " + + maxSize + " )"); } } diff --git a/src/share/classes/sun/awt/image/ShortInterleavedRaster.java b/src/share/classes/sun/awt/image/ShortInterleavedRaster.java index ef624f978..c55d111d7 100644 --- a/src/share/classes/sun/awt/image/ShortInterleavedRaster.java +++ b/src/share/classes/sun/awt/image/ShortInterleavedRaster.java @@ -171,7 +171,7 @@ public class ShortInterleavedRaster extends ShortComponentRaster { sampleModel); } this.bandOffset = this.dataOffsets[0]; - verify(false); + verify(); } /** @@ -762,33 +762,6 @@ public class ShortInterleavedRaster extends ShortComponentRaster { return createCompatibleWritableRaster(width,height); } - /** - * Verify that the layout parameters are consistent with - * the data. If strictCheck - * is false, this method will check for ArrayIndexOutOfBounds conditions. If - * strictCheck is true, this method will check for additional error - * conditions such as line wraparound (width of a line greater than - * the scanline stride). - * @return String Error string, if the layout is incompatible with - * the data. Otherwise returns null. - */ - private void verify (boolean strictCheck) { - int maxSize = 0; - int size; - - for (int i=0; i < numDataElements; i++) { - size = (height-1)*scanlineStride + (width-1)*pixelStride + - dataOffsets[i]; - if (size > maxSize) { - maxSize = size; - } - } - if (data.length < maxSize) { - throw new RasterFormatException("Data array too small (should be "+ - maxSize+" )"); - } - } - public String toString() { return new String ("ShortInterleavedRaster: width = "+width +" height = " + height diff --git a/src/share/native/sun/awt/image/awt_parseImage.c b/src/share/native/sun/awt/image/awt_parseImage.c index 264a02f16..cdd22189d 100644 --- a/src/share/native/sun/awt/image/awt_parseImage.c +++ b/src/share/native/sun/awt/image/awt_parseImage.c @@ -114,6 +114,62 @@ int awt_parseImage(JNIEnv *env, jobject jimage, BufImageS_t **imagePP, return status; } +/* Verifies whether the channel offsets are sane and correspond to the type of + * the raster. + * + * Return value: + * 0: Failure: channel offsets are invalid + * 1: Success + */ +static int checkChannelOffsets(RasterS_t *rasterP, int dataArrayLength) { + int i, lastPixelOffset, lastScanOffset; + switch (rasterP->rasterType) { + case COMPONENT_RASTER_TYPE: + if (!SAFE_TO_MULT(rasterP->height, rasterP->scanlineStride)) { + return 0; + } + if (!SAFE_TO_MULT(rasterP->width, rasterP->pixelStride)) { + return 0; + } + + lastScanOffset = (rasterP->height - 1) * rasterP->scanlineStride; + lastPixelOffset = (rasterP->width - 1) * rasterP->pixelStride; + + + if (!SAFE_TO_ADD(lastPixelOffset, lastScanOffset)) { + return 0; + } + + lastPixelOffset += lastScanOffset; + + for (i = 0; i < rasterP->numDataElements; i++) { + int off = rasterP->chanOffsets[i]; + int size = lastPixelOffset + off; + + if (off < 0 || !SAFE_TO_ADD(lastPixelOffset, off)) { + return 0; + } + + if (size < lastPixelOffset || size >= dataArrayLength) { + // an overflow, or insufficient buffer capacity + return 0; + } + } + return 1; + case BANDED_RASTER_TYPE: + // NB:caller does not support the banded rasters yet, + // so this branch of the code must be re-defined in + // order to provide valid criteria for the data offsets + // verification, when/if banded rasters will be supported. + // At the moment, we prohibit banded rasters as well. + return 0; + default: + // PACKED_RASTER_TYPE: does not support channel offsets + // UNKNOWN_RASTER_TYPE: should not be used, likely indicates an error + return 0; + } +} + /* Parse the raster. All of the raster information is returned in the * rasterP structure. * @@ -125,7 +181,6 @@ int awt_parseImage(JNIEnv *env, jobject jimage, BufImageS_t **imagePP, int awt_parseRaster(JNIEnv *env, jobject jraster, RasterS_t *rasterP) { jobject joffs = NULL; /* int status;*/ - int isDiscrete = TRUE; if (JNU_IsNull(env, jraster)) { JNU_ThrowNullPointerException(env, "null Raster object"); @@ -155,6 +210,9 @@ int awt_parseRaster(JNIEnv *env, jobject jraster, RasterS_t *rasterP) { return -1; } + // make sure that the raster type is initialized + rasterP->rasterType = UNKNOWN_RASTER_TYPE; + if (rasterP->numBands <= 0 || rasterP->numBands > MAX_NUMBANDS) { @@ -254,7 +312,6 @@ int awt_parseRaster(JNIEnv *env, jobject jraster, RasterS_t *rasterP) { } rasterP->chanOffsets[0] = (*env)->GetIntField(env, jraster, g_BPRdataBitOffsetID); rasterP->dataType = BYTE_DATA_TYPE; - isDiscrete = FALSE; } else { rasterP->type = sun_awt_image_IntegerComponentRaster_TYPE_CUSTOM; @@ -265,7 +322,19 @@ int awt_parseRaster(JNIEnv *env, jobject jraster, RasterS_t *rasterP) { return 0; } - if (isDiscrete) { + // do basic validation of the raster structure + if (rasterP->width <= 0 || rasterP->height <= 0 || + rasterP->pixelStride <= 0 || rasterP->scanlineStride <= 0) + { + // invalid raster + return -1; + } + + // channel (data) offsets + switch (rasterP->rasterType) { + case COMPONENT_RASTER_TYPE: + case BANDED_RASTER_TYPE: // note that this routine does not support banded rasters at the moment + // get channel (data) offsets rasterP->chanOffsets = NULL; if (SAFE_TO_ALLOC_2(rasterP->numDataElements, sizeof(jint))) { rasterP->chanOffsets = @@ -278,10 +347,21 @@ int awt_parseRaster(JNIEnv *env, jobject jraster, RasterS_t *rasterP) { } (*env)->GetIntArrayRegion(env, joffs, 0, rasterP->numDataElements, rasterP->chanOffsets); + if (rasterP->jdata == NULL) { + // unable to verify the raster + return -1; + } + // verify whether channel offsets look sane + if (!checkChannelOffsets(rasterP, (*env)->GetArrayLength(env, rasterP->jdata))) { + return -1; + } + break; + default: + ; // PACKED_RASTER_TYPE does not use the channel offsets. } - /* additioanl check for sppsm fields validity: make sure that - * size of raster samples doesn't exceed the data type cpacity. + /* additional check for sppsm fields validity: make sure that + * size of raster samples doesn't exceed the data type capacity. */ if (rasterP->dataType > UNKNOWN_DATA_TYPE && /* data type has been recognized */ rasterP->sppsm.maxBitSize > 0 && /* raster has SPP sample model */ diff --git a/src/share/native/sun/awt/medialib/safe_alloc.h b/src/share/native/sun/awt/medialib/safe_alloc.h index ce744af2c..579d98638 100644 --- a/src/share/native/sun/awt/medialib/safe_alloc.h +++ b/src/share/native/sun/awt/medialib/safe_alloc.h @@ -41,5 +41,10 @@ (((w) > 0) && ((h) > 0) && ((sz) > 0) && \ (((0xffffffffu / ((juint)(w))) / ((juint)(h))) > ((juint)(sz)))) +#define SAFE_TO_MULT(a, b) \ + (((a) > 0) && ((b) >= 0) && ((0x7fffffff / (a)) > (b))) + +#define SAFE_TO_ADD(a, b) \ + (((a) >= 0) && ((b) >= 0) && ((0x7fffffff - (a)) > (b))) #endif // __SAFE_ALLOC_H__ -- GitLab