New subject: [Bug 25691] ImageList_Duplicate doesn't correctly duplicate full length of has_alpha byte array

4 Jan 2011


      http://bugs.winehq.org/show_bug.cgi?id=25691
Summary: ImageList_Duplicate doesn't correctly duplicate full
                    length of has_alpha byte array
           Product: Wine
           Version: unspecified
          Platform: x86
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: comctl32
        AssignedTo: wine-bugs@winehq.org
        ReportedBy: rmcdonald@bittorrent.com
In the case where an image list is created, then expanded enough so that the
byte array pointed at by the has_alpha member has been reallocated to a larger
size (and cMaxImage is larger), if that image list is duplicated, the duplicate
will have a has_alpha member that is shorter than the expected length.
Where ImageList_Create assigns:
himl->cMaxImage = cInitial + 1;
and then ensures the length of the has_alpha byte array is at least cMaxImage
bytes long,
ImageList_Duplicate assigns to the duplicate:
himlDst->cMaxImage = himlSrc->cMaxImage;
which throws off the internal checks that assume the length of the has_alpha
member is at least as long as cMaxImage says.  This results in heap corruption
when the following line in add_with_alpha() is executed under the right
conditions:
himl->has_alpha[pos + n] = 1;
I worked around the bug by making the following changes to
dlls/comctl32/imagelist.c:
*** 1519,1524 ****
--- 1519,1537 ----
                      himlSrc->hdcMask, 0, 0, SRCCOPY);
himlDst->cCurImage = himlSrc->cCurImage;
+         if (himlSrc->has_alpha && himlDst->has_alpha
+             && himlSrc->cMaxImage > himlDst->cMaxImage)
+         {
+             /* ImageList_Create didn't create a long enough new_alpha */
+             char *new_alpha = HeapReAlloc( GetProcessHeap(),
HEAP_ZERO_MEMORY,
+                 himlDst->has_alpha, himlSrc->cMaxImage );
+             if (new_alpha) himlDst->has_alpha = new_alpha;
+             else
+             {
+                 HeapFree( GetProcessHeap(), 0, himlDst->has_alpha );
+                 himlDst->has_alpha = NULL;
+             }
+         }
      himlDst->cMaxImage = himlSrc->cMaxImage;
          if (himlSrc->has_alpha && himlDst->has_alpha)
              memcpy( himlDst->has_alpha, himlSrc->has_alpha,
himlDst->cCurImage );
I built Wine 1.2.2 from source (./configure; make; sudo make install), and then
applied the changes the same way and retested.
Looks like the 1.3 and development lines have the same issue.
I don't know if this patch is efficient enough for me to submit it (nicer if
ImageList_Create would accept cMaxImage as a parameter so it could create the
byte array with the correct length to save an allocation, but that's a bigger
change).
-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.

[Bug 25691] New: ImageList_Duplicate doesn't correctly duplicate full length of has_alpha byte array