Re: inline pixbufs

[Darin Adler <darin eazel com>]

on 7/26/00 12:04 PM, Havoc Pennington at hp@redhat.com wrote:

>> I look forward to helping out by reviewing your code to see if I can spot
>> any missing cases, since there are many interactions between the various
>> parameters.
> 
> Please do. My approach is to just reject anything that isn't RGB or
> RGBA, and barf if width > rowstride, and verify that the length is at
> least rowstride*height. I think the interactions don't get too nasty
> until we support another format or two.

I found two things.

> +        if (length >= 0 &&
> +            length < (height * rowstride + 13)) {
> +                /* Not enough buffer to hold the remaining header
> +                 * information plus the data.
> +                 */
> +                
> +                return NULL;
> +        }

Need to handle overflow here. If a garbled height or row stride is a very
large value, then length might look good even though it is too short.

> +        /* Read the remaining 13 bytes of header information */
> +            
> +        has_alpha = read_bool (&p);
> +        colorspace = read_int (&p);
> +        n_channels = read_int (&p);
> +        bits_per_sample = read_int (&p);
> +
> +        if (colorspace != GDK_COLORSPACE_RGB)
> +                return NULL;
> +
> +        if (bits_per_sample != 8)
> +                return NULL;
> +
> +        if (n_channels < 3 || n_channels > 4)
> +                return NULL;

Could have a check here that has_alpha is consistent with n_channels. Does
it work to have n_channels == 4 and has_alpha == false?

One way to do it is:

           if (n_channels != (has_alpha ? 4 : 3))
                   return NULL;

Also, someone pedantic might suggest that you reject booleans that are
anything other than 0 or 1, rather than treating all non-0 as 1, but that
person would be way more pedantic than me :-)

    -- Darin