Remove four useless checks in dlls/gdi32/enhmetafile.c (RESEND)

[Gerald Pfeifer]

On Sun, 2 Dec 2007, Alexandre Julliard wrote:
> I'm aware of that, but the purpose of having these warnings is to spot
> bugs, and when you find a bug you have to fix it. Yes, the checks
> currently don't work, so they should be made to work, not removed. As
> the comment says, you have to check that offsets and sizes are contained
> within the record.

Doesn't the following part of the checks work and ensure that already?

        if ( !( ((lpCreate->offBmi +lpCreate->cbBmi ) <= mr->nSize) &&
                ((lpCreate->offBits+lpCreate->cbBits) <= mr->nSize) ) )

My patch only removes the half of the original check which is a noop 
(since the current code  misses the fact that these variables always
will be >= 0 by virtue of their type).

Ahhh!  A lightbulb goes on.  Since this is input from the outside, and
thus completely out of our control, you are worried about overflows, that
is, the sum of the two values (offset and size) being within range, but
not the individual parts.  

Is this what you've been after? :-)

Updated patch below.  (Now I only wonder whether the <= in the original
code shouldn't have been <, and thus the > in my code shouldn't be >=,
but that's a separate issue.)

Gerald

Index: dlls/gdi32/enhmetafile.c
===================================================================
RCS file: /home/wine/wine/dlls/gdi32/enhmetafile.c,v
retrieving revision 1.6
diff -u -3 -p -r1.6 enhmetafile.c
--- dlls/gdi32/enhmetafile.c	3 Aug 2007 13:06:43 -0000	1.6
+++ dlls/gdi32/enhmetafile.c	2 Dec 2007 23:32:50 -0000
@@ -1669,11 +1669,15 @@ BOOL WINAPI PlayEnhMetaFileRecord(
         const EMRCREATEDIBPATTERNBRUSHPT *lpCreate = (const EMRCREATEDIBPATTERNBRUSHPT *)mr;
         LPVOID lpPackedStruct;
 
-        /* check that offsets and data are contained within the record */
-        if ( !( (lpCreate->cbBmi>=0) && (lpCreate->cbBits>=0) &&
-                (lpCreate->offBmi>=0) && (lpCreate->offBits>=0) &&
-                ((lpCreate->offBmi +lpCreate->cbBmi ) <= mr->nSize) &&
-                ((lpCreate->offBits+lpCreate->cbBits) <= mr->nSize) ) )
+        /* Check that offsets and data are contained within the record.
+         * The first four checks are not redundant -- think overflow!
+         */
+        if ( lpCreate->offBmi     > mr->nSize 
+             || lpCreate->cbBmi   > mr->nSize
+             || lpCreate->offBits > mr->nSize
+             || lpCreate->cbBits  > mr->nSize
+             || lpCreate->offBmi +lpCreate->cbBmi  > mr->nSize
+             || lpCreate->offBits+lpCreate->cbBits > mr->nSize )
         {
             ERR("Invalid EMR_CREATEDIBPATTERNBRUSHPT record\n");
             break;