Skip to content

nsgifload: ensure GIF_INSUFFICIENT_FRAME_DATA is unrecoverable #2166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

lovell
Copy link
Member

@lovell lovell commented Mar 28, 2021

As discussed in #1709 (comment), this should fix all of the OSS Fuzz issues found so far today (currently 10).

@lovell lovell requested a review from jcupitt March 28, 2021 14:00
@jcupitt
Copy link
Member

jcupitt commented Mar 28, 2021

This feels very severe to me -- it'll block all truncated GIFs, and that's a common case that used to be handled.

Is there some other way we can fix this? Perhaps some sanity checking on the dimensions?

@kleisauke
Copy link
Member

Looks like the parameters of the calloc call in vips_foreign_load_nsgif_bitmap_create is overflowing. Here's the salient part of that OSS-Fuzz issue:

ERROR: AddressSanitizer: calloc parameters overflow: count * size (-1643047120 * 4) cannot be represented in type size_t (thread T0)
<snip />
  | #1 0x787bab in vips_foreign_load_nsgif_bitmap_create libvips/libvips/foreign/nsgifload.c:510:16
<snip />

Perhaps a sanity check would help there?

@jcupitt
Copy link
Member

jcupitt commented Mar 28, 2021

Oh, good idea, perhaps

        if( width <= 0 ||
                width > VIPS_MAX_COORD ||
                height <= 0 ||
                height > VIPS_MAX_COORD ) {
                vips_error ...
                return( NULL );
        }

        return calloc( (size_t) width * height, 4 );

Though it would still be easy to bust through the 2gb fuzz limit, of course.

@lovell
Copy link
Member Author

lovell commented Mar 28, 2021

Thanks for the feedback, take 2 of this PR at #2168

@lovell lovell closed this Mar 28, 2021
@lovell lovell deleted the nsgifload-fail-insufficient-frame-data branch March 29, 2021 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants