From eefcd374a1f034485840315fcc6a9c80a14489ff Mon Sep 17 00:00:00 2001 From: fvogel Date: Fri, 2 Jun 2017 20:48:47 +0000 Subject: Fixed [b601ce3ab1]: A corrupted image can cause resource exhaustion. Patch for core-8-5-branch from Keith Nash --- generic/tkImgGIF.c | 1 + generic/tkImgPhoto.c | 28 +++++---- tests/imgPhoto.test | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 10 deletions(-) diff --git a/generic/tkImgGIF.c b/generic/tkImgGIF.c index e576559..409300c 100644 --- a/generic/tkImgGIF.c +++ b/generic/tkImgGIF.c @@ -1393,6 +1393,7 @@ Fread( } memcpy(dst, handle->data, (size_t) (hunk * count)); handle->data += hunk * count; + handle->length -= hunk * count; return (int)(hunk * count); } diff --git a/generic/tkImgPhoto.c b/generic/tkImgPhoto.c index f6fee84..4e1aa01 100644 --- a/generic/tkImgPhoto.c +++ b/generic/tkImgPhoto.c @@ -3008,6 +3008,10 @@ ImgPhotoSetSize( height = masterPtr->userHeight; } + if (width > INT_MAX / 4) { + /* Pitch overflows int */ + return TCL_ERROR; + } pitch = width * 4; /* @@ -3023,6 +3027,10 @@ ImgPhotoSetSize( unsigned /*long*/ newPixSize = (unsigned /*long*/) (height * pitch); + if (pitch && height > (int)(UINT_MAX / pitch)) { + return TCL_ERROR; + } + /* * Some mallocs() really hate allocating zero bytes. [Bug 619544] */ @@ -3073,14 +3081,14 @@ ImgPhotoSetSize( if ((masterPtr->pix32 != NULL) && ((width == masterPtr->width) || (width == validBox.width))) { if (validBox.y > 0) { - memset(newPix32, 0, (size_t) (validBox.y * pitch)); + memset(newPix32, 0, ((size_t) validBox.y * pitch)); } h = validBox.y + validBox.height; if (h < height) { - memset(newPix32 + h*pitch, 0, (size_t) ((height - h) * pitch)); + memset(newPix32 + h*pitch, 0, ((size_t) (height - h) * pitch)); } } else { - memset(newPix32, 0, (size_t) (height * pitch)); + memset(newPix32, 0, ((size_t) height * pitch)); } if (masterPtr->pix32 != NULL) { @@ -3097,7 +3105,7 @@ ImgPhotoSetSize( offset = validBox.y * pitch; memcpy(newPix32 + offset, masterPtr->pix32 + offset, - (size_t) (validBox.height * pitch)); + ((size_t) validBox.height * pitch)); } else if ((validBox.width > 0) && (validBox.height > 0)) { /* @@ -3108,7 +3116,7 @@ ImgPhotoSetSize( srcPtr = masterPtr->pix32 + (validBox.y * masterPtr->width + validBox.x) * 4; for (h = validBox.height; h > 0; h--) { - memcpy(destPtr, srcPtr, (size_t) (validBox.width * 4)); + memcpy(destPtr, srcPtr, ((size_t) validBox.width * 4)); destPtr += width * 4; srcPtr += masterPtr->width * 4; } @@ -3266,7 +3274,7 @@ ImgPhotoInstanceSetSize( if (masterPtr->width == instancePtr->width) { offset = validBox.y * masterPtr->width * 3; memcpy(newError + offset, instancePtr->error + offset, - (size_t) (validBox.height + ((size_t) validBox.height * masterPtr->width * 3 * sizeof(schar))); } else if (validBox.width > 0 && validBox.height > 0) { @@ -4419,7 +4427,7 @@ Tk_PhotoPutBlock( && (blockPtr->pitch == pitch))) && (compRule == TK_PHOTO_COMPOSITE_SET)) { memmove(destLinePtr, blockPtr->pixelPtr + blockPtr->offset[0], - (size_t) (height * width * 4)); + ((size_t) height * width * 4)); /* * We know there's an alpha offset and we're setting the data, so skip @@ -4451,7 +4459,7 @@ Tk_PhotoPutBlock( && (blueOffset == 2) && (alphaOffset == 3) && (width <= blockPtr->width) && compRuleSet) { - memcpy(destLinePtr, srcLinePtr, (size_t) (width * 4)); + memcpy(destLinePtr, srcLinePtr, ((size_t) width * 4)); srcLinePtr += blockPtr->pitch; destLinePtr += pitch; continue; @@ -5425,12 +5433,12 @@ Tk_PhotoBlank( */ memset(masterPtr->pix32, 0, - (size_t) (masterPtr->width * masterPtr->height * 4)); + ((size_t) masterPtr->width * masterPtr->height * 4)); for (instancePtr = masterPtr->instancePtr; instancePtr != NULL; instancePtr = instancePtr->nextPtr) { if (instancePtr->error) { memset(instancePtr->error, 0, - (size_t) (masterPtr->width * masterPtr->height + ((size_t) masterPtr->width * masterPtr->height * 3 * sizeof(schar))); } } diff --git a/tests/imgPhoto.test b/tests/imgPhoto.test index 14c3d40..f1b0e68 100644 --- a/tests/imgPhoto.test +++ b/tests/imgPhoto.test @@ -27,6 +27,25 @@ README -- Tk test suite design document. set teapotPhotoFile [file join [file dirname [info script]] teapot.ppm] testConstraint hasTeapotPhoto [file exists $teapotPhotoFile] +# - Tests 18.1-18.9 cause segfault on Tk 8.5 < 8.5.20. +proc test18ok {} { + expr { + ([package vsatisfies $::tk_patchLevel 8.6]) + || ( ($::tk_version eq "8.5") + && ([package vcompare $::tk_patchLevel 8.5.20] == 1) + ) + } +} + +proc base64ok {} { + expr { + ![catch {package require base64}] + } +} + +testConstraint SegfaultOn8.5 [test18ok] +testConstraint base64PackageNeeded [base64ok] + test imgPhoto-1.1 {options for photo images} { image create photo p1 -width 79 -height 83 list [lindex [p1 configure -width] 4] [lindex [p1 configure -height] 4] \ @@ -724,6 +743,143 @@ test imgPhoto-16.1 {copying to self doesn't access freed memory} { image delete $i } {} +# Reject corrupted or truncated image [Bug b601ce3ab1]. +# WARNING - tests marked "SegfaultOn8.5" will cause a segfault on +# 8.5.19 and lower. +test imgPhoto-18.1 {Reject corrupted GIF (binary string)} -constraints { + SegfaultOn8.5 base64PackageNeeded +} -setup { + package require base64 + set data [base64::decode { + R0lGODlhwjMz//8zM/8z/zP/MzP/////M////yH5CiwheLrcLTBCd6Tv2qW16tdK4jhV + 5qpraXIvM1JlNyAgOw== + }] +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {error reading color map} +test imgPhoto-18.2 {Reject corrupted GIF (base 64 string)} -constraints { + SegfaultOn8.5 +} -setup { + set data { + R0lGODlhwjMz//8zM/8z/zP/MzP/////M////yH5CiwheLrcLTBCd6Tv2qW16tdK4jhV + 5qpraXIvM1JlNyAgOw== + } +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {error reading color map} +test imgPhoto-18.3 {Reject corrupted GIF (file)} -constraints { + SegfaultOn8.5 +} -setup { + set fileName [file join [file dirname [info script]] corruptMangled.gif] +} -body { + image create photo gif1 -file $fileName +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {error reading color map} +test imgPhoto-18.4 {Reject truncated GIF (binary string)} -constraints { + SegfaultOn8.5 base64PackageNeeded +} -setup { + package require base64 + set data [base64::decode { + R0lGODlhEAAQAMIHAAAAADMz//8zM/8z/zP/MzP///8= + }] +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {error reading color map} +test imgPhoto-18.5 {Reject truncated GIF (base 64 string)} -constraints { + SegfaultOn8.5 +} -setup { + set data { + R0lGODlhEAAQAMIHAAAAADMz//8zM/8z/zP/MzP///8= + } +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {error reading color map} +test imgPhoto-18.6 {Reject truncated GIF (file)} -constraints { + SegfaultOn8.5 +} -setup { + set fileName [file join [file dirname [info script]] corruptTruncated.gif] +} -body { + image create photo gif1 -file $fileName +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {error reading color map} +test imgPhoto-18.7 {Reject corrupted GIF (> 4Gb) (binary string)} -constraints { + SegfaultOn8.5 base64PackageNeeded +} -setup { + package require base64 + set data [base64::decode { + R0lGODlhwmYz//8zM/8z/zP/MzP/////M////yH5Ciwhe + LrcLTBCd6Tv2qW16tdK4jhV5qpraXIvM1JlNyAgOw== + }] +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {not enough free memory for image buffer} +test imgPhoto-18.8 {Reject corrupted GIF (> 4Gb) (base 64 string)} -constraints { + SegfaultOn8.5 +} -setup { + set data { + R0lGODlhwmYz//8zM/8z/zP/MzP/////M////yH5Ciwhe + LrcLTBCd6Tv2qW16tdK4jhV5qpraXIvM1JlNyAgOw== + } +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {not enough free memory for image buffer} +test imgPhoto-18.9 {Reject corrupted GIF (> 4Gb) (file)} -constraints { + SegfaultOn8.5 +} -setup { + set fileName [file join [file dirname [info script]] corruptMangled4G.gif] +} -body { + image create photo gif1 -file $fileName +} -cleanup { + catch {image delete gif1} +} -returnCodes error -result {not enough free memory for image buffer} +test imgPhoto-18.10 {Valid GIF (binary string)} -constraints { + base64PackageNeeded +} -setup { + # Test the binary string reader with a valid GIF. + # This is not tested elsewhere. + # Tests 18.11, 18.12, with matching data, are included for completeness. + package require base64 + set data [base64::decode { + R0lGODlhEAAQAMIHAAAAADMz//8zM/8z/zP/MzP/////M////yH5BAEKAAcALAAA + AAAQABAAAAMheLrcLTBCd6QV79qlterXB0riOFXmmapraXIvM1IdZTcJADs= + }] +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -result gif1 +test imgPhoto-18.11 {Valid GIF (base 64 string)} -setup { + set data { + R0lGODlhEAAQAMIHAAAAADMz//8zM/8z/zP/MzP/////M////yH5BAEKAAcALAAA + AAAQABAAAAMheLrcLTBCd6QV79qlterXB0riOFXmmapraXIvM1IdZTcJADs= + } +} -body { + image create photo gif1 -data $data +} -cleanup { + catch {image delete gif1} +} -result gif1 +test imgPhoto-18.12 {Valid GIF (file)} -setup { + set fileName [file join [file dirname [info script]] red.gif] +} -body { + image create photo gif1 -file $fileName +} -cleanup { + catch {image delete gif1} +} -result gif1 + destroy .c eval image delete [image names] -- cgit v0.12