Skip to content

heifsave: limit output dimensions #3513

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 5, 2023
Merged

heifsave: limit output dimensions #3513

merged 2 commits into from
Jun 5, 2023

Conversation

lovell
Copy link
Member

@lovell lovell commented May 28, 2023

Here's a possible fix for #3509 targetting the 8.14 branch

It takes a similar approach as already used in webpsave:

libvips/libvips/foreign/webpsave.c

Lines 672 to 675 in 0df1fc5

if( save->ready->Xsize > 16383 || page_height > 16383 ) {
vips_error( "webpsave", _( "image too large" ) );
vips_foreign_save_webp_unset( webp );
return( -1 );

The arbitrary 32768x32768 limit is to match the libheif default input limit. However, libvips' heifload operation defaults to a limit of 16384x16384, or 65535x65535 when using unlimited. Thoughts here welcome, as I can happily be convinced to use alternative arbitrary limits instead.

@lovell lovell mentioned this pull request May 28, 2023
Closed
@kleisauke
Copy link
Member

I think this will still cause a int overflow, see for example:

Details 
# https://github.com/libvips/libvips/issues/3509
$ vips heifsave test.v out.avif --lossless --effort 9 --Q 62
../libvips/foreign/heifsave.c:404:34: runtime error: signed integer overflow: 8286 * 259200 cannot be represented in type 'int'
    #0 0x7f0e5f44cf6c in vips_foreign_save_heif_write_block /home/kleisauke/libvips/libvips/foreign/heifsave.c:404:34
    #1 0x7f0e647839eb in wbuffer_write /home/kleisauke/libvips/libvips/iofuncs/sinkdisc.c:174:25
    #2 0x7f0e6478363e in wbuffer_write_thread /home/kleisauke/libvips/libvips/iofuncs/sinkdisc.c:199:3
    #3 0x7f0e646d2f95 in vips_threadset_work /home/kleisauke/libvips/libvips/iofuncs/threadset.c:134:3
    #4 0x7f0e646cedff in vips_thread_run /home/kleisauke/libvips/libvips/iofuncs/thread.c:148:11
    #5 0x7f0e63541892  (/lib64/libglib-2.0.so.0+0x8a892) (BuildId: b0e6a618cd46494b058c5f00ce2f1a650b200ce3)
    #6 0x7f0e62eae906 in start_thread (/lib64/libc.so.6+0x8c906) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
    #7 0x7f0e62f3486f in __GI___clone3 (/lib64/libc.so.6+0x11286f) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../libvips/foreign/heifsave.c:404:34 in

# w <= 32768 && h <= 32768
$ vips black x.avif[effort=0,Q=1] 32768 32768
../libvips/foreign/heifsave.c:404:34: runtime error: signed integer overflow: 21846 * 98304 cannot be represented in type 'int'
    #0 0x7f965584d28c in vips_foreign_save_heif_write_block /home/kleisauke/libvips/libvips/foreign/heifsave.c:404:34
    #1 0x7f965ab839eb in wbuffer_write /home/kleisauke/libvips/libvips/iofuncs/sinkdisc.c:174:25
    #2 0x7f965ab8363e in wbuffer_write_thread /home/kleisauke/libvips/libvips/iofuncs/sinkdisc.c:199:3
    #3 0x7f965aad2f95 in vips_threadset_work /home/kleisauke/libvips/libvips/iofuncs/threadset.c:134:3
    #4 0x7f965aacedff in vips_thread_run /home/kleisauke/libvips/libvips/iofuncs/thread.c:148:11
    #5 0x7f9659941892  (/lib64/libglib-2.0.so.0+0x8a892) (BuildId: b0e6a618cd46494b058c5f00ce2f1a650b200ce3)
    #6 0x7f96592ae906 in start_thread (/lib64/libc.so.6+0x8c906) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
    #7 0x7f965933486f in __GI___clone3 (/lib64/libc.so.6+0x11286f) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../libvips/foreign/heifsave.c:404:34 in 

# w * h <= INT_MAX / 3
$ vips black x.avif[effort=0,Q=1] 26754 26754
../libvips/foreign/heifsave.c:404:34: runtime error: signed integer overflow: 26753 * 80272 cannot be represented in type 'int'
    #0 0x7fce86c742bc in vips_foreign_save_heif_write_block /home/kleisauke/libvips/libvips/foreign/heifsave.c:404:34
    #1 0x7fce8bf839eb in wbuffer_write /home/kleisauke/libvips/libvips/iofuncs/sinkdisc.c:174:25
    #2 0x7fce8bf8363e in wbuffer_write_thread /home/kleisauke/libvips/libvips/iofuncs/sinkdisc.c:199:3
    #3 0x7fce8bed2f95 in vips_threadset_work /home/kleisauke/libvips/libvips/iofuncs/threadset.c:134:3
    #4 0x7fce8becedff in vips_thread_run /home/kleisauke/libvips/libvips/iofuncs/thread.c:148:11
    #5 0x7fce8ad41892  (/lib64/libglib-2.0.so.0+0x8a892) (BuildId: b0e6a618cd46494b058c5f00ce2f1a650b200ce3)
    #6 0x7fce8a6ae906 in start_thread (/lib64/libc.so.6+0x8c906) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)
    #7 0x7fce8a73486f in __GI___clone3 (/lib64/libc.so.6+0x11286f) (BuildId: 245240a31888ad5c11bbc55b18e02d87388f59a9)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../libvips/foreign/heifsave.c:404:34 in 

# w * h <= INT_MAX / 4
$ vips black x.avif[effort=0,Q=1] 23170 23170 && echo 'OK'
OK

Perhaps we should also limit width * height to INT_MAX / 4 (i.e. similar to cgifsave)?:

libvips/libvips/foreign/cgifsave.c

Line 680 in fcf3ff3

if( (guint64) cgif->frame_width * cgif->frame_height > INT_MAX / 4 ||

@lovell lovell force-pushed the 8.14-heifsave-limit-dimensions branch from 4e72334 to 5325742 Compare June 4, 2023 07:57
@lovell
Copy link
Member Author

lovell commented Jun 4, 2023

Thanks, good spot, updated.

@kleisauke
Copy link
Member

For reference, it looks like libavif (and thus Chromium-based browsers) will limit AVIF image size to 16384x16384.
https://github.com/AOMediaCodec/libavif/blob/454adbf5c27d24e1645cae53c1c659d46e07f657/include/avif/avif.h#L71-L76

For 32-bit systems, the image size in dav1d is limited to 8192x8192 (Chromium no longer uses libaom for AVIF decoding).
https://code.videolan.org/videolan/dav1d/-/blob/8b419c16bf1e37bc98044089da58f06824462cb9/src/lib.c#L197-206

@lovell lovell force-pushed the 8.14-heifsave-limit-dimensions branch from 5325742 to 904ee98 Compare June 4, 2023 20:36
@lovell @kleisauke
Update libvips/foreign/heifsave.c
12a1b0c 
Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>
@kleisauke kleisauke merged commit 462dc2d into libvips:8.14 Jun 5, 2023
@kleisauke
Copy link
Member

Ah, I accidentally removed the referring PR within the commit message when I landed this. Sorry for that.

@lovell lovell deleted the 8.14-heifsave-limit-dimensions branch June 5, 2023 14:17
@lovell
Copy link
Member Author

lovell commented Jun 5, 2023

I forgot to add a changelog entry for this, sorry.

@diegoatpitch diegoatpitch mentioned this pull request May 2, 2024
Closed
@kleisauke kleisauke mentioned this pull request Feb 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kleisauke kleisauke kleisauke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lovell @kleisauke