[Bug 21613] New: Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

Summary: Stack buffer read overflow in
                    GdipCreateBitmapFromScan0
           Product: Wine
           Version: 1.0.0
          Platform: x86
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdiplus
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

The gdiplus image tests have long shown the following error in valgrind:
 Conditional jump or move depends on uninitialised value(s)
    at  X11DRV_DIB_GenColorMap (dib.c:367)
    by  X11DRV_DIB_BuildColorMap (dib.c:436)
    by  X11DRV_CreateDIBSection (dib.c:4742)
    by  CreateDIBSection (dib.c:1353)
    by  GdipCreateBitmapFromScan0 (image.c:1251)
    by  GdipCreateBitmapFromHBITMAP (image.c:2782)
    by  test_GdipCreateBitmapFromHBITMAP (image.c:512)
    by  func_image (image.c:1523)
    by  run_test (test.h:535)
    by  main (test.h:585)
  Uninitialised value was created by a stack allocation
    at  GdipCreateBitmapFromScan0 (image.c:1204)
e.g. http://kegel.com/wine/valgrind/logs/2010-01-04-10.14/vg-gdiplus_image.txt

Turns out, even though biClrUsed is zero, 
CreateDibSection expects the BITMAPINFO to have a valid color table;
see gdi32/dib.c line 1161:

1160         colors = info->bmiHeader.biClrUsed;
   1161         if (!colors) colors = 1 << info->bmiHeader.biBitCount;

The attached patch gets rid of the warning, and lets the tests pass,
but seems wrong; the color table probably needs to have real values.

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

--- Comment #1 from Dan Kegel   2010-02-06 10:19:03 ---
Created an attachment (id=26082)
 --> (http://bugs.winehq.org/attachment.cgi?id=26082)
Patch adding color table to bitmapinfo, but filling with zeroes instead of real
values

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

--- Comment #2 from Vincent Povirk   2010-02-06 10:39:04 ---
I've been avoiding this for a while because I wanted to get the indexed color
stuff right. Gdiplus now stores its own color table for indexed color bitmaps,
but it doesn't sync the table with gdi32.

I don't think there's any reason to do this. I'm guessing gdi32 can't draw from
or to these bitmaps properly, since the palette has an alpha channel. The only
reason to keep making DIBs for them is so that we can get a (broken) device
context for the bitmaps and prevent any program that needs to draw to them from
crashing. In the long run, we should draw to them without using gdi32, and then
we can stop making DIBs.

My guess is that programs generally don't draw to indexed color bitmaps.
They're crippled on native gdiplus 1.0 and just not the sort of thing you use
gdiplus for. Even indexed color image files are really loaded as RGB color.

I'd be inclined to send a patch that sets a DIB color table of 1 color,
regardless of the actual depth.

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

--- Comment #3 from Dan Kegel   2010-02-06 12:16:26 ---
That might be better than the random values we're reading
from the stack now.  Seems like the current code should be
failing some test, though.  Probably the thing to do is
improve our tests a bit so they show this flaw (without
Valgrind) first.

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|1.0.0                       |1.1.38

--- Comment #4 from Dmitry Timoshkov   2010-02-09 01:49:06 ---
Assuming it's really not 1.0.0

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

Dan Kegel  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|1.1.38                      |1.0.0

--- Comment #5 from Dan Kegel   2010-02-09 08:56:39 ---
That bug's been there a long time.

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

--- Comment #6 from Vincent Povirk   2010-03-30 17:25:36 ---
This should be fixed now.

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #7 from Austin English  2011-03-24 17:37:33 CDT ---
My latest valgrind run (1.3.16) hung in mshtml, but the gdiplus portion of the
log shows no valgrind warnings, so marking fixed.

[Bug 21613] Stack buffer read overflow in GdipCreateBitmapFromScan0

http://bugs.winehq.org/show_bug.cgi?id=21613

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #8 from Alexandre Julliard  2011-04-01 12:40:47 CDT ---
Closing bugs fixed in 1.3.17.