Skip to content

Commit

Permalink
Image: Keep track of contiguous buffer size to avoid heap buffer over…
Browse files Browse the repository at this point in the history
…flows

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.
  • Loading branch information
reuk committed Feb 23, 2022
1 parent ec86769 commit 0223e44
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ class UnityPeer : public ComponentPeer,
{
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;
Expand Down
4 changes: 3 additions & 1 deletion modules/juce_graphics/images/juce_Image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ class SoftwarePixelData : public ImagePixelData

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;
Expand Down
1 change: 1 addition & 0 deletions modules/juce_graphics/images/juce_Image.h
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ class JUCE_API Image final
Rectangle<int> getBounds() const noexcept { return Rectangle<int> (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. */
Expand Down
28 changes: 14 additions & 14 deletions modules/juce_graphics/native/juce_mac_CoreGraphicsContext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ Agreement and JUCE Privacy Policy (both effective as of the 16th June 2020).

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;
Expand Down Expand Up @@ -111,22 +113,20 @@ static CGImageRef getCachedImageRef (const Image& juceImage, CGColorSpaceRef col
static CGImageRef createImage (const Image& juceImage, CGColorSpaceRef colourSpace)
{
const Image::BitmapData srcData (juceImage, Image::BitmapData::readOnly);
detail::DataProviderPtr provider;

if (auto* cgim = dynamic_cast<CoreGraphicsPixelData*> (juceImage.getPixelData()))
const auto provider = [&]
{
provider = detail::DataProviderPtr { CGDataProviderCreateWithData (new ImageDataContainer::Ptr (cgim->imageData),
if (auto* cgim = dynamic_cast<CoreGraphicsPixelData*> (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<CFDataRef> data (CFDataCreate (nullptr,
(const UInt8*) srcData.data,
(CFIndex) ((size_t) srcData.lineStride * (size_t) srcData.height)));
provider = detail::DataProviderPtr { CGDataProviderCreateWithCFData (data.get()) };
}
}

CFUniquePtr<CFDataRef> 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,
Expand Down Expand Up @@ -512,7 +512,7 @@ static CGBitmapInfo getCGImageFlags (const Image::PixelFormat& format)

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());
Expand Down
4 changes: 3 additions & 1 deletion modules/juce_gui_basics/native/juce_android_Windowing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,9 @@ class AndroidComponentPeer : public ComponentPeer,
bm.lineStride = width * static_cast<int> (sizeof (jint));
bm.pixelStride = static_cast<int> (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
Expand Down
4 changes: 3 additions & 1 deletion modules/juce_gui_basics/native/juce_win32_Windowing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,9 @@ class WindowsBitmapImage : public ImagePixelData

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,9 @@ class XBitmapImage : public ImagePixelData
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;
Expand Down
3 changes: 3 additions & 0 deletions modules/juce_opengl/opengl/juce_OpenGLImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ class OpenGLFrameBufferImage : public ImagePixelData
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);
Expand Down

0 comments on commit 0223e44

Please sign in to comment.