[Bug 54741] New: integer overflow in get_dib_stride / NtGdiCreateBitmap
https://bugs.winehq.org/show_bug.cgi?id=54741 Bug ID: 54741 Summary: integer overflow in get_dib_stride / NtGdiCreateBitmap Product: Wine Version: 8.4 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: gdi32 Assignee: wine-bugs(a)winehq.org Reporter: felix-wine(a)fefe.de Distribution: --- Here's the source code of NtGdiCreateBitmap: 97 HBITMAP WINAPI NtGdiCreateBitmap( INT width, INT height, UINT planes, 98 UINT bpp, const void *bits ) 99 { Having INT for width and height is already bad, but can't be helped because you are implementing a crappy API. 105 if (width > 0x7ffffff || height > 0x7ffffff) 106 { 107 RtlSetLastWin32Error( ERROR_INVALID_PARAMETER ); 108 return 0; 109 } 110 111 if (!width || !height) 112 return 0; 113 114 if (height < 0) 115 height = -height; 116 if (width < 0) 117 width = -width; After this the value of width and height is 1..0x7ffffff (7 digits, not 8). bpp is validated to be at most 32. 140 dib_stride = get_dib_stride( width, bpp ); 141 size = dib_stride * height; 142 /* Check for overflow (dib_stride itself must be ok because of the constraint on bm.bmWidth above). */ 143 if (dib_stride != size / height) Here's the code of get_dib_stride: 282 static inline int get_dib_stride( int width, int bpp ) 283 { 284 return ((width * bpp + 31) >> 3) & ~3; 285 } width can be 0x7ffffff, bpp can be 32. width*bpp is then 0xffffffe0 which looks at first glance like no overflow happened, but the type is still int, i.e. signed 32-bit, i.e. max value is 0x7fffffff (8 digits), max bpp to trigger that would be 16 not 32. We are right-shifting afterwards, so we ought to be fine? Nope, because we are operating on signed ints, so it's an arithmetic right shift, not a logical one. The & does not remove the sign bit. So what happens here is that get_dib_stride can return a negative value. OK but NtGdiCreateBitmap checks for overflow and will notice something is wrong. Not really. If width=height=0x7ffffff and bpp=32 then get_dib_stride=-4. Then size is -4*0x7ffffff promoted to size_t, or 0xe0000004. The overflow check does this: 143 if (dib_stride != size / height) size/0x7ffffff is 0xfffffffc. dib_stride is -4, promoted to size_t for the comparison is also 0xfffffffc. Also: On the way we had undefined behavior. The compiler would be allowed to do all kinds of nasty things. -- 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.
participants (1)
-
WineHQ Bugzilla