From 0223e44ae75193015b4fc86aa0e9d4347b62eb2c Mon Sep 17 00:00:00 2001 From: reuk Date: Thu, 10 Feb 2022 18:59:38 +0000 Subject: [PATCH] Image: Keep track of contiguous buffer size to avoid heap buffer overflows In CoreGraphicsPixelData::createImage, image data was copied from a BitmapData created from the Image passed into the function. The BitmapData instance didn't keep track of the size of the buffer it pointed to, so the buffer size was computed by multiplying the BitmapData height by its line stride. However, if the BitmapData pointed to a subsection of an image, the `data` pointer might be offset from the allocated region, and `data + lineStride * height` would point past the end of the allocated region. Trying to read/copy this range would cause a heap buffer overflow at the end of the range. This change adjusts BitmapData so that it keeps track of the size of the allocated region. Taking a subsection of an image should subtract the data pointer offset from the size of the allocated region. --- .../Unity/juce_Unity_Wrapper.cpp | 4 ++- modules/juce_graphics/images/juce_Image.cpp | 4 ++- modules/juce_graphics/images/juce_Image.h | 1 + .../native/juce_mac_CoreGraphicsContext.mm | 28 +++++++++---------- .../native/juce_android_Windowing.cpp | 4 ++- .../native/juce_win32_Windowing.cpp | 4 ++- .../native/x11/juce_linux_XWindowSystem.cpp | 4 ++- .../juce_opengl/opengl/juce_OpenGLImage.cpp | 3 ++ 8 files changed, 33 insertions(+), 19 deletions(-) diff --git a/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp b/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp index 753af3127f..7aae5b91e9 100644 --- a/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp +++ b/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp @@ -162,7 +162,9 @@ private: { ignoreUnused (mode); - bitmap.data = imageData + x * pixelStride + y * lineStride; + const auto offset = (size_t) x * (size_t) pixelStride + (size_t) y * (size_t) lineStride; + bitmap.data = imageData + offset; + bitmap.size = (size_t) (lineStride * height) - offset; bitmap.pixelFormat = pixelFormat; bitmap.lineStride = lineStride; bitmap.pixelStride = pixelStride; diff --git a/modules/juce_graphics/images/juce_Image.cpp b/modules/juce_graphics/images/juce_Image.cpp index 10213980ed..c4355fe78b 100644 --- a/modules/juce_graphics/images/juce_Image.cpp +++ b/modules/juce_graphics/images/juce_Image.cpp @@ -97,7 +97,9 @@ public: void initialiseBitmapData (Image::BitmapData& bitmap, int x, int y, Image::BitmapData::ReadWriteMode mode) override { - bitmap.data = imageData + (size_t) x * (size_t) pixelStride + (size_t) y * (size_t) lineStride; + const auto offset = (size_t) x * (size_t) pixelStride + (size_t) y * (size_t) lineStride; + bitmap.data = imageData + offset; + bitmap.size = (size_t) (height * lineStride) - offset; bitmap.pixelFormat = pixelFormat; bitmap.lineStride = lineStride; bitmap.pixelStride = pixelStride; diff --git a/modules/juce_graphics/images/juce_Image.h b/modules/juce_graphics/images/juce_Image.h index 714472304f..62bb19fd27 100644 --- a/modules/juce_graphics/images/juce_Image.h +++ b/modules/juce_graphics/images/juce_Image.h @@ -349,6 +349,7 @@ public: Rectangle getBounds() const noexcept { return Rectangle (width, height); } uint8* data; /**< The raw pixel data, packed according to the image's pixel format. */ + size_t size; /**< The number of valid/allocated bytes after data. May be smaller than "lineStride * height" if this is a section of a larger image. */ PixelFormat pixelFormat; /**< The format of the data. */ int lineStride; /**< The number of bytes between each line. */ int pixelStride; /**< The number of bytes between each pixel. */ diff --git a/modules/juce_graphics/native/juce_mac_CoreGraphicsContext.mm b/modules/juce_graphics/native/juce_mac_CoreGraphicsContext.mm index 941862203b..df5f9f9487 100644 --- a/modules/juce_graphics/native/juce_mac_CoreGraphicsContext.mm +++ b/modules/juce_graphics/native/juce_mac_CoreGraphicsContext.mm @@ -71,7 +71,9 @@ public: void initialiseBitmapData (Image::BitmapData& bitmap, int x, int y, Image::BitmapData::ReadWriteMode mode) override { - bitmap.data = imageData->data + x * pixelStride + y * lineStride; + const auto offset = (size_t) (x * pixelStride + y * lineStride); + bitmap.data = imageData->data + offset; + bitmap.size = (size_t) (lineStride * height) - offset; bitmap.pixelFormat = pixelFormat; bitmap.lineStride = lineStride; bitmap.pixelStride = pixelStride; @@ -111,22 +113,20 @@ public: static CGImageRef createImage (const Image& juceImage, CGColorSpaceRef colourSpace) { const Image::BitmapData srcData (juceImage, Image::BitmapData::readOnly); - detail::DataProviderPtr provider; - if (auto* cgim = dynamic_cast (juceImage.getPixelData())) + const auto provider = [&] { - provider = detail::DataProviderPtr { CGDataProviderCreateWithData (new ImageDataContainer::Ptr (cgim->imageData), + if (auto* cgim = dynamic_cast (juceImage.getPixelData())) + { + return detail::DataProviderPtr { CGDataProviderCreateWithData (new ImageDataContainer::Ptr (cgim->imageData), srcData.data, - (size_t) srcData.lineStride * (size_t) srcData.height, + srcData.size, [] (void * __nullable info, const void*, size_t) { delete (ImageDataContainer::Ptr*) info; }) }; - } - else - { - CFUniquePtr data (CFDataCreate (nullptr, - (const UInt8*) srcData.data, - (CFIndex) ((size_t) srcData.lineStride * (size_t) srcData.height))); - provider = detail::DataProviderPtr { CGDataProviderCreateWithCFData (data.get()) }; - } + } + + CFUniquePtr data (CFDataCreate (nullptr, (const UInt8*) srcData.data, (CFIndex) srcData.size)); + return detail::DataProviderPtr { CGDataProviderCreateWithCFData (data.get()) }; + }(); CGImageRef imageRef = CGImageCreate ((size_t) srcData.width, (size_t) srcData.height, @@ -512,7 +512,7 @@ void CoreGraphicsContext::drawImage (const Image& sourceImage, const AffineTrans auto colourSpace = sourceImage.getFormat() == Image::PixelFormat::SingleChannel ? greyColourSpace.get() : rgbColourSpace.get(); - auto image = detail::ImagePtr { CoreGraphicsPixelData::getCachedImageRef (sourceImage, colourSpace) }; + detail::ImagePtr image { CoreGraphicsPixelData::getCachedImageRef (sourceImage, colourSpace) }; ScopedCGContextState scopedState (context.get()); CGContextSetAlpha (context.get(), state->fillType.getOpacity()); diff --git a/modules/juce_gui_basics/native/juce_android_Windowing.cpp b/modules/juce_gui_basics/native/juce_android_Windowing.cpp index 3c4074b0df..7609d0181f 100644 --- a/modules/juce_gui_basics/native/juce_android_Windowing.cpp +++ b/modules/juce_gui_basics/native/juce_android_Windowing.cpp @@ -1213,7 +1213,9 @@ private: bm.lineStride = width * static_cast (sizeof (jint)); bm.pixelStride = static_cast (sizeof (jint)); bm.pixelFormat = Image::ARGB; - bm.data = (uint8*) (data + x + y * width); + const auto offset = (size_t) x + (size_t) y * (size_t) width; + bm.data = (uint8*) (data + offset); + bm.size = sizeof (jint) * (((size_t) height * (size_t) width) - offset); } ImagePixelData::Ptr clone() override diff --git a/modules/juce_gui_basics/native/juce_win32_Windowing.cpp b/modules/juce_gui_basics/native/juce_win32_Windowing.cpp index 5066806812..b2520020b3 100644 --- a/modules/juce_gui_basics/native/juce_win32_Windowing.cpp +++ b/modules/juce_gui_basics/native/juce_win32_Windowing.cpp @@ -990,7 +990,9 @@ public: void initialiseBitmapData (Image::BitmapData& bitmap, int x, int y, Image::BitmapData::ReadWriteMode mode) override { - bitmap.data = imageData + x * pixelStride + y * lineStride; + const auto offset = (size_t) (x * pixelStride + y * lineStride); + bitmap.data = imageData + offset; + bitmap.size = (size_t) (lineStride * height) - offset; bitmap.pixelFormat = pixelFormat; bitmap.lineStride = lineStride; bitmap.pixelStride = pixelStride; diff --git a/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp b/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp index d1d7674170..059e98e25d 100644 --- a/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp +++ b/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp @@ -968,7 +968,9 @@ public: void initialiseBitmapData (Image::BitmapData& bitmap, int x, int y, Image::BitmapData::ReadWriteMode mode) override { - bitmap.data = imageData + x * pixelStride + y * lineStride; + const auto offset = (size_t) (x * pixelStride + y * lineStride); + bitmap.data = imageData + offset; + bitmap.size = (size_t) (lineStride * height) - offset; bitmap.pixelFormat = pixelFormat; bitmap.lineStride = lineStride; bitmap.pixelStride = pixelStride; diff --git a/modules/juce_opengl/opengl/juce_OpenGLImage.cpp b/modules/juce_opengl/opengl/juce_OpenGLImage.cpp index 716cec6473..709b6778da 100644 --- a/modules/juce_opengl/opengl/juce_OpenGLImage.cpp +++ b/modules/juce_opengl/opengl/juce_OpenGLImage.cpp @@ -168,6 +168,9 @@ private: bitmapData.dataReleaser.reset (r); bitmapData.data = (uint8*) r->data.get(); + bitmapData.size = (size_t) bitmapData.width + * (size_t) bitmapData.height + * sizeof (PixelARGB); bitmapData.lineStride = (bitmapData.width * bitmapData.pixelStride + 3) & ~3; ReaderType::read (frameBuffer, bitmapData, x, y);