Hi,
thanks for your attention to this.
Bas Zoetekouw wrote:
>> diff -u imlib2-1.4.0/src/modules/loaders/loader_xpm.c
>> imlib2-1.4.0/src/modules/loaders/loader_xpm.c
>> --- imlib2-1.4.0/src/modules/loaders/loader_xpm.c
>> +++ imlib2-1.4.0/src/modules/loaders/loader_xpm.c
>> @@ -246,8 +246,8 @@
>> return 0;
>> }
>> ptr = im->data;
>> - end = ptr + (sizeof(DATA32) * w * h);
>> pixels = w * h;
>> + end = ptr + pixels;
>> }
>> else
>> {
>
> Are you sure this patch actually fixes the bug reported here? I agree
> that the use of sizeof(DATA32) here is definately a bug and should be
> fixed, but I'm not sure that that's all there is to it.
>
> The reporter of the bug as well as the CVE say the actual problem here
> is that the height and width are read from the header, and might not be
> the actual size of the picture being loaded. I don't see how this patch
> fixes that issue (although must confess I haven't looked at the code in
> detail).
>
> Concretely: can't w*h still overflow in the code above, for a suitably crafted
> header?
ptr and end are both DATA32*.
ptr is initialized to
im->data = (DATA32 *) malloc(sizeof(DATA32) * im->w * im->h);
(im->w and im->h are equal to w and h, respectively) and later on this
is used as
for (i = 0;
((i < 65536) && (ptr < end) && (line[i]));
i++)
{
...
*ptr++ = ...
}
I think this should be OK even end ends up < ptr because some one of w,h
is negative for some reason or an overflow, but I'm more happy to be
corrected than have imlib2 in the release with an incorrect patch. :)
Kind regards
T.
--
Thomas Viehmann, http://thomas.viehmann.net/
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]
