We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 724

Summary: SDL_bmp.c crashes on headers with negative height values
Product: SDL Reporter: Eric Wing <ewing.bug.sdl>
Component: videoAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: critical    
Priority: P2 CC: ewmailing, renesd, sezeroz
Version: 1.2.13Keywords: target-1.2.14
Hardware: All   
OS: All   
URL: http://www.assembla.com/spaces/SDL_Clipboard
Attachments: negative height/width in bmp loading.
fixes to SDL_bmp.c for negative height

Description Eric Wing 2009-04-07 08:07:11 UTC 
SDL_bmp.c does not handle BMP headers with negative height values. I discovered this problem while working on a copy & paste API for SDL. I discovered that Apple tends to create BMP headers that have negative values for height to represent that an image is inverted. This negative value messes up a bunch of calculations in SDL and usually causes a crash in memory allocation.

This is a problem for reading BMP files written by OS X and also reading BMPs from memory that were generated within OS X (e.g. pasteboard).


I tried to track down more information about the use of negative values in BMP headers. All I found was the following from the Cocoa mailing list responding to somebody who also ran into this problem with ImageIO:

<quote>
John Stiles <JStiles@blizzard.com> 	Fri, Jan 11, 2008 at 2:34 AM
Subject: BMP bug in ImageIO?
Cc: cocoa-dev@lists.apple.com
A negative height in the BMP header means that the scanlines are stored from top-to-bottom (rather than bottom-to-top, which is what you get if you put a positive height). I don't believe that this is a bug.
</quote>


The attached file is my modified version from the SDL 1.2.13 official source release. I assume SDL 1.3 also needs a similar fix.

My fix looks for negative height values and flips the sign. I also invert the image. I also recommend that a public API to invert an image be added to SDL 1.3 as it is really handy for this kind of stuff and OpenGL stuff. (And with a public API, writing optimizations might finally make sense, e.g. using Apple's vImage on OS X.)


If you need a reproducible test case to crash on, you can use my current work for the SDL clipboard at:
http://www.assembla.com/spaces/SDL_Clipboard
When building/running on a Mac, pasting an image from the pasteboard will result in a crash (click the bottom-right quadrant).
Comment 1 Ryan C. Gordon 2009-09-13 16:33:14 UTC 
Tagging this bug with "target-1.2.14" so we can try to resolve it for SDL 1.2.14.

Please note that we may choose to resolve it as WONTFIX. This tag is largely so we have a comprehensive wishlist of bugs to examine for 1.2.14 (and so we can close bugs that we'll never fix, rather than have them live forever in Bugzilla).

--ryan.
Comment 2 Rene Dudfield 2009-09-17 05:03:23 UTC 
Created attachment 359 [details]
negative height/width in bmp loading.

ah, this explains my issues with SDL and OSX bmp files.  This is an important fix for pygame too.

There's no patch attached to this bug.  I assume this was forgotten to be attached?

I've attached an SDL_bmp2.c that 'Kenneth Bull' sent to the mailing list.


"""
Subject: Re: [SDL] SDL_Loadbmp fail

Try the attached file (not thoroughly tested, but compiles without warnings
with gcc -c SDL_bmp2.c `pkg-config sdl --cflags`).

I renamed SDL_LoadBMP_RW and SDL_SaveBMP_RW so you can test it without
recompiling SDL.  This is for 1.2, not 1.3.

This should also work better for BITMAPV4HEADER and BITMAPV5HEADER bitmaps.
"""
Comment 3 Rene Dudfield 2009-09-17 05:04:53 UTC 
Updated to critical, as this is also a security vulnerability.
Comment 4 Eric wing 2009-09-17 11:14:21 UTC 
Created attachment 360 [details]
fixes to SDL_bmp.c for negative height
Comment 5 Eric wing 2009-09-17 11:15:53 UTC 
Sorry, I forgot to explicitly state where my fix was located. It is in my aforementioned repository under the FixesToSDL subdirectory.

http://trac-hg.assembla.com/SDL_Clipboard/browser/FixesToSDL/SDL_bmp.c

I have attached my file here for convenience.
Comment 6 Sam Lantinga 2009-09-26 04:44:07 UTC 
This is fixed for SDL 1.2, 1.3, and SDL_image.

Thanks!