Skip to content

vipsload: sanity check resolution #2742

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

Merged
merged 1 commit into from
Apr 3, 2022

Conversation

lovell
Copy link
Member

@lovell lovell commented Apr 2, 2022

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46009 (when compiled with debug assertions).

Before:

$ vipsheader -a fail.v

(vipsheader:15655): VIPS-WARNING **: 10:35:38.096: error reading vips image metadata: VipsImage: XML parse error

sanity failure: VipsImage (0x558a4f1da330) bad resolution

**
VIPS:ERROR:../libvips/iofuncs/image.c:3619:vips_image_pio_input: assertion failed: (vips_object_sanity( VIPS_OBJECT( image ) ))
Bail out! VIPS:ERROR:../libvips/iofuncs/image.c:3619:vips_image_pio_input: assertion failed: (vips_object_sanity( VIPS_OBJECT( image ) ))
Aborted (core dumped)

After:

$ vipsheader -a fail.v
vipsheader: VipsImage: invalid resolution
VipsImage: unable to read header for "fail.v"

@lovell
Copy link
Member Author

lovell commented Apr 2, 2022

Looks like the CIFuzz job failed for an unrelated reason (perhaps a build failure that needs fixing downstream in OSS Fuzz?).

@kleisauke
Copy link
Member

[100%] Linking CXX shared library libjxl_extras_dec.so

It seems that libjxl builds shared libraries despite passing the -DBUILD_SHARED_LIBS=0 CMake option (since commit libjxl/libjxl@775d010).
https://github.com/libjxl/libjxl/blob/79d85890e7b409b4f76db019e2d0f8d21a354cb5/lib/jxl_extras.cmake#L176-L178

I'm preparing a PR to resolve this in libjxl. The latest libvips' OSS-Fuzz build also seems to fail due to this.
https://oss-fuzz-build-logs.storage.googleapis.com/index.html#libvips

@kleisauke
Copy link
Member

PR libjxl/libjxl#1315 should fix the failing CIFuzz and OSS-Fuzz builds.

@jcupitt
Copy link
Member

jcupitt commented Apr 2, 2022

In exif load we do:

        image->Xres = VIPS_MAX( 0, xres );
        image->Yres = VIPS_MAX( 0, yres );

Perhaps the behaviour for vips load should be the same?

@lovell lovell force-pushed the vips-sanity-check-resolution branch from b1196c9 to 0c04ac1 Compare April 2, 2022 19:32
@lovell
Copy link
Member Author

lovell commented Apr 2, 2022

Thanks John, makes sense, I've updated to do so. This approach keeps things consistent with vips_image_init_fields too.

@jcupitt jcupitt merged commit 9228e50 into libvips:master Apr 3, 2022
@jcupitt
Copy link
Member

jcupitt commented Apr 3, 2022

Great!

@lovell lovell deleted the vips-sanity-check-resolution branch April 3, 2022 15:55
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