[PATCH 3/8] Xephyr: integer overflow in ephyrHostGLXGetStringFromServer()

[Alan Coopersmith]

On 07/ 6/13 02:01 AM, walter harms wrote:
>> @@ -169,7 +169,8 @@ ephyrHostGLXGetStringFromServer(int a_screen_number,
>>       int default_screen = DefaultScreen(dpy);
>>       xGLXGenericGetStringReq *req = NULL;
>>       xGLXSingleReply reply;
>> -    int length = 0, numbytes = 0, major_opcode = 0, get_string_op = 0;
>> +    unsigned long length = 0, numbytes = 0;
>> +    int major_opcode = 0, get_string_op = 0;
>>
>>       EPHYR_RETURN_VAL_IF_FAIL(dpy && a_string, FALSE);
>>
>> @@ -209,36 +210,46 @@ ephyrHostGLXGetStringFromServer(int a_screen_number,
>>
>>       _XReply(dpy, (xReply *) &reply, 0, False);
>>
>> -    length = reply.length * 4;
>> -    if (!length) {
>> -        numbytes = 0;
>> -    }
>> -    else {
>> +#if UINT32_MAX >= (ULONG_MAX / 4)
>> +    if (reply.length >= (ULONG_MAX / 4))
>> +        goto eat_out;
>> +#endif
>
> I am not sure what is going on here, i am missing the else branch,
> For all systems where UINT == ULONG this will be a noop. Is that intended ?

reply.length is a CARD32 (aka uint32_t).   We need to ensure that when we
multiply it by 4, it will fit into the length variable, which is changed
to a unsigned long.

In the 32-bit machine case, where unsigned longs are the same size as uint32_t,
we make this check for overflow.   We don't need an else branch for the #if,
because if the unsigned long is able to hold values at least as large as 4
times the UINT32_MAX (as it will be on LP64 systems), there is no possibility
of overflow.

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc