winegcc: sign-compare fixes

[richardvoigt at gmail.com]

> > >  memmove(&arr->base[i], &arr->base[i + 1],
> > (arr->size - i - 1) *
> > > sizeof(arr->base[0]));
> > >  arr->size--;
> > >  }
> >
> > If the element size could be greater than one byte,
> > the multiplication
> > here should be checked for overflow (actually, this
> > is more necessary
> > when the array is created or expanded).
> >
> > >
> >
>
> Well, the base field in the structure is defined as
> "const char** base" so sizeof(base[0]) == sizeof(const
> char *), should be 4 or so.
> How do you mean overflow in this case? sorry for
> asking dumb questions...
>
One desires that the argument is the size of memory which holds
numelems = (arr->size - i - 1) elements of each = sizeof
(arr->base[0]) bytes each.  As you say each = 4 bytes on 32-bit
systems.

Then the value is numelems * each = numelems * 4 = numelems << 2.
Thus if either of the two most significant bits in numelems are set,
they will be lost as a result of the shift (multiplication).  In this
case the correct value is larger than addressable virtual memory,
which should be illegal.  Actually though, the limit should be placed
on arr->size in the allocation function.

The essential problem here is that this can be used to remove sanity
checking in the case where the inputs are supplied by a client with
lesser permissions.  Imagine, for example, a kernel function being
invoked via an ioctl... though the problem is equally valid for any
network-facing service.

Say a client is allowed to create and access buffer for exchange with
a device (USB printer or the like), and that the client can allocate
the buffer by providing the number of buffer elements, and the element
size is fixed and non-unity, we'll use 4 in this example.  If the
privileged service does something like the following:

handle createbuffer(int buflen)
{
    if (buflen <= 0) return NULL
    int bytecount = 4 * buflen;
    if (bytecount > quota) return NULL;
    int* p = malloc(bytecount);
    if (!p) return NULL;
    return createbufferhandle(buflen, p);
}

and lets the client read from the buffer:
int readbufferatindex(handle buffer, int index)
{
    int size = buffersizefromhandle(buffer);
    if (index < 0 || index >= size) return 0;
    return bufferptrfromhandle(buffer)[index];
}

Seems safe enough, right?  Each client is allowed only a limited
buffer, maybe elsewhere the number of buffers is limited, so the
client can't execute a DOS attack.  malloc guarantees that the buffer
doesn't overlap any other buffer, and the bounds checking in
readbufferatindex prevents clients from reading outside their buffer.

Until I do this:
buffer = createbuffer(0x40000010);

Uh-oh.  I passed the quota check, because bytecount was calculated at
64 bytes.  malloc easily allocated 64 bytes not overlapping with any
other user.  But the stored buffersize is set so large that the bounds
check is worthless; I can read any memory I want.