CoreGraphicsImage imageData size calculation mistake

1

We had a user loading a 32000 × 19202 JPEG Image. Although we encouraged the user to scale his image down, his image resulted in a crash in our application.
The problem originated from the CoreGraphicsImage, it was trying to allocate it’s HeapBlock with an incorrect size.
This is why:
CoreGraphicsImage constructor:
…
imageData.allocate ((size_t) (lineStride * jmax (1, height)), clearImage);

But it should actually be this:
imageData.allocate ((size_t)lineStride * jmax (1, height), clearImage);

1 Like
2

Oops, well also in CoreGraphicsImage::createImage the calculation is incorrect.
But after fixing all cases i still experience a crash when loading and drawing an image with these dimensions.

So in short, guys can you help me out?
If i load this huge image, JUCE will crash.
It will only crash if you fix the size calculation, if not the size becomes so large (overflow) that it will fail allocating the memory for the bitmapData, so it won’t try to do anything. So although the code looks safe it’s actually wrong.

When calculation is fixed, bitmapData will be valid and Image is loaded.
But when i try to draw it, it fails in CoreGraphicsContext::drawImage.
As you see in the snippet i’m actually scaling the image. If i just call drawImageAt(image, 0,0); it actually works fine.

Here is the code snippet.
HUGE.jpg is an Image with size 32000 × 19202

Image image = ImageFileFormat::loadFrom(File("HUGE.jpg"));
if (image.isValid())
{
    Image thumb(Image::ARGB, 160, 120, true);

    Graphics g(thumb);
    g.setColour(Colours::white);

    g.drawImageWithin( image, 0, 0, thumb.getWidth(), thumb.getHeight(), RectanglePlacement( RectanglePlacement::centred ), false );
    g.drawRect(0, 0, thumb.getWidth(), thumb.getHeight(), 10.0);
}

Here is the file:
HUGE JPEG

3

Fixed:

There were a couple of other places where we were overflowing integers.

2 Likes
4

Huge is the right word for it! Amazed that it works at all with images that big!

5

Thanx t0m for looking at it. Don’t forget the SoftwarePixelData class, it has the same issues.
@jules: Yes huge it is. I will need to put some hardcoded resolution restrains on image loading. Because with the fixes you guys made the Image class handles the data just fine. But because of that the image is actually being drawn and that in turn causes a crash.

6

I can draw the image, and a resized version of it, without a crash…

7

@t0m: i just checked out the develop branch. I took the AnimationAppExample and changed the MainComponent to this. It crashes in void CoreGraphicsContext::drawImage at line 528. CGContextDrawImage (context, imageRect, image);
It’s quite possible this issue is related to the user’s hardware. I would appreciate it if you could test it again on another machine than your own. For me it’s a certain crash each time this code is run.

class MainContentComponent   : public AnimatedAppComponent
{
public:
	Image image;
	
//==============================================================================
MainContentComponent()
{
    setSize (800, 600);
    setFramesPerSecond (60);
		image = ImageFileFormat::loadFrom(File("/Users/edwindekoning/Downloads/HUGE.jpg"));
		
}

void update() override
{
    // This function is called at the frequency specified by the setFramesPerSecond() call
    // in the constructor. You can use it to update counters, animate values, etc.
}

void paint (Graphics& g) override
{
		if (image.isValid())
		{
			g.drawImageWithin( image, 0, 0, getWidth(), getHeight(), RectanglePlacement( RectanglePlacement::centred ), false );
		}
}

void resized() override
{
    // This is called when the MainContentComponent is resized.
    // If you add any child components, this is where you should
    // update their positions.
}


private:
//==============================================================================

// Your private member variables go here...


JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (MainContentComponent)
};

machinespecs

8

You’re trying to animate that at 60 frames per second?! The call to .drawImageWithin takes about 5 seconds on my machine…

1 Like
9

It’s working on a mid 2010 Macbook Pro running 10.12.6 and mid 2015 Macbook Pro running 10.13.1.

10

Could you post the stacktrace of the crash?

11

Here is the stack trace:

StackTrace
StackTrace.png731×543 104 KB

12

I’ve managed to generate a crash on a 10.11.6 MacMini with Xcode 8.1.

13

It appears to be an issue on 10.11, rather than hardware - I updated the MacMini to 10.13 to get better debugging tools and can no longer reproduce the problem.

Unfortunately everything looked correct when calling CGContextDrawImage and I didn’t find anything more informative, so I can’t suggest anything useful.

14
1 Like
15

I had the feeling it wasn’t really something we could fix easily. Thanx for looking into it though.

16

I ran into this issue today again because someone send us a JPEG with a resolution of 30000x24000 :wink:
So i tried to load this file using the JUCE Image class but the JPEGImageFormat crashes because it uses the BitmapData getLinePointer and that performs explicit casts and thus cause an integer overflow.

So i would suggest the following changes:

    inline uint8* getLinePointer (int y) const noexcept                 { return data + (size_t)y * (size_t)lineStride; }

    inline uint8* getPixelPointer (int x, int y) const noexcept         { return data + (size_t)y * (size_t)lineStride + (size_t)x * (size_t)pixelStride; }
17

Bump, the BitmapData::getLinePointer and BitmapData::getPixelPointer are faulty. They have integer overflow issues. You fixed similar issues in the Image class a while ago but without these fixes it’s still broken.

18

Thanks! I’ve just got around to adding this in 383d69c.

19

Thank you!

20

Another offender is in Image::moveImageSection

const size_t lineSize = (size_t)(destData.pixelStride * w);

Should become

const size_t lineSize = (size_t)destData.pixelStride * (size_t)w;
1 Like