fix an off by one error in token get#4105
Merged
Merged
Conversation
Member
|
It looks like the issue isn't fully resolved yet. I encountered a similar issue while fuzzing on this branch locally. Details$ vips copy test/test-suite/images/sample.png x.png[Q=75]\'x\'
=================================================================
==102648==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50200008f57f at pc 0x7f0d2bc417fa bp 0x7ffe7fb7d220 sp 0x7ffe7fb7d218
READ of size 1 at 0x50200008f57f thread T0
#0 0x7f0d2bc417f9 in vips__token_get /home/kleisauke/libvips/build/../libvips/iofuncs/util.c:1303:8
#1 0x7f0d2bc3dfa8 in vips__find_rightmost_brackets /home/kleisauke/libvips/build/../libvips/iofuncs/util.c:1482:9
#2 0x7f0d2bc3dcf1 in vips_filename_suffix_match /home/kleisauke/libvips/build/../libvips/iofuncs/util.c:498:20
#3 0x7f0d2b3d4961 in vips_foreign_find_save_target_sub /home/kleisauke/libvips/build/../libvips/foreign/foreign.c:2128:3
#4 0x7f0d2bc3bed6 in vips_slist_map2 /home/kleisauke/libvips/build/../libvips/iofuncs/util.c:101:33
#5 0x7f0d2b3d469a in vips_foreign_map /home/kleisauke/libvips/build/../libvips/foreign/foreign.c:489:11
#6 0x7f0d2b3d469a in vips_foreign_find_save_target /home/kleisauke/libvips/build/../libvips/foreign/foreign.c:2154:46
#7 0x7f0d2bb98ae8 in vips_image_write_to_file /home/kleisauke/libvips/build/../libvips/iofuncs/image.c:2692:19
#8 0x7f0d2bb83b99 in vips_object_get_argument_to_string /home/kleisauke/libvips/build/../libvips/iofuncs/object.c:2243:8
#9 0x7f0d2bbdc884 in vips_call_argv_output /home/kleisauke/libvips/build/../libvips/iofuncs/operation.c:1420:8
#10 0x7f0d2bb76c1c in vips_argument_map /home/kleisauke/libvips/build/../libvips/iofuncs/object.c:612:17
#11 0x7f0d2bbdbefb in vips_call_argv /home/kleisauke/libvips/build/../libvips/iofuncs/operation.c:1487:6
#12 0x5067f5 in main /home/kleisauke/libvips/build/../tools/vips.c:884:7
#13 0x7f0d2a839087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 77c77fee058b19c6f001cf2cb0371ce3b8341211)
#14 0x7f0d2a83914a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 77c77fee058b19c6f001cf2cb0371ce3b8341211)
#15 0x42a6e4 in _start (/usr/bin/vips+0x42a6e4) (BuildId: b3e63d689b6f578d2d6a7b733a3d2d4cc7ddf057)
0x50200008f57f is located 0 bytes after 15-byte region [0x50200008f570,0x50200008f57f)
allocated by thread T0 here:
#0 0x4c5533 in malloc (/usr/bin/vips+0x4c5533) (BuildId: b3e63d689b6f578d2d6a7b733a3d2d4cc7ddf057)
#1 0x7f0d2c574879 in g_malloc (/lib64/libglib-2.0.so.0+0x63879) (BuildId: 36b60dbd02e796145a982d0151ce37202ec05649)
#2 0x7f0d2c553e0e in g_path_get_basename (/lib64/libglib-2.0.so.0+0x42e0e) (BuildId: 36b60dbd02e796145a982d0151ce37202ec05649)
#3 0x7f0d2bc3dce5 in vips_filename_suffix_match /home/kleisauke/libvips/build/../libvips/iofuncs/util.c:494:13
#4 0x7f0d2b3d4961 in vips_foreign_find_save_target_sub /home/kleisauke/libvips/build/../libvips/foreign/foreign.c:2128:3
#5 0x7f0d2bc3bed6 in vips_slist_map2 /home/kleisauke/libvips/build/../libvips/iofuncs/util.c:101:33
#6 0x7f0d2b3d469a in vips_foreign_map /home/kleisauke/libvips/build/../libvips/foreign/foreign.c:489:11
#7 0x7f0d2b3d469a in vips_foreign_find_save_target /home/kleisauke/libvips/build/../libvips/foreign/foreign.c:2154:46
#8 0x7f0d2bb98ae8 in vips_image_write_to_file /home/kleisauke/libvips/build/../libvips/iofuncs/image.c:2692:19
#9 0x7f0d2bb83b99 in vips_object_get_argument_to_string /home/kleisauke/libvips/build/../libvips/iofuncs/object.c:2243:8
#10 0x7f0d2bbdc884 in vips_call_argv_output /home/kleisauke/libvips/build/../libvips/iofuncs/operation.c:1420:8
#11 0x7f0d2bb76c1c in vips_argument_map /home/kleisauke/libvips/build/../libvips/iofuncs/object.c:612:17
#12 0x7f0d2bbdbefb in vips_call_argv /home/kleisauke/libvips/build/../libvips/iofuncs/operation.c:1487:6
#13 0x5067f5 in main /home/kleisauke/libvips/build/../tools/vips.c:884:7
#14 0x7f0d2a839087 in __libc_start_call_main (/lib64/libc.so.6+0x2a087) (BuildId: 77c77fee058b19c6f001cf2cb0371ce3b8341211)
#15 0x7f0d2a83914a in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2a14a) (BuildId: 77c77fee058b19c6f001cf2cb0371ce3b8341211)
#16 0x42a6e4 in _start (/usr/bin/vips+0x42a6e4) (BuildId: b3e63d689b6f578d2d6a7b733a3d2d4cc7ddf057)
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/kleisauke/libvips/build/../libvips/iofuncs/util.c:1303:8 in vips__token_get
Shadow bytes around the buggy address:
0x50200008f280: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
0x50200008f300: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
0x50200008f380: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
0x50200008f400: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
0x50200008f480: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
=>0x50200008f500: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00[07]
0x50200008f580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x50200008f600: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x50200008f680: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x50200008f700: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x50200008f780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==102648==ABORTING
Aborted (core dumped) |
Member
Author
|
Yes, I'm adding some tests and revising the code again. There will be another version sigh. |
Member
Author
|
I added tests via a tiny C prog and some shell script, rather than via pyvips. It means we have a new item in the exposed API, which is unfortunate for a stable release :( Perhaps the tests should be moved to master, and 8.15 should only have the fixed-up |
Member
Author
|
... I moved the tests to master in another PR to avoid changing the stable ABI. |
Member
Author
|
Tests are now here #4113 |
kleisauke
approved these changes
Aug 24, 2024
kleisauke
left a comment
Member
There was a problem hiding this comment.
I confirm that this resolves the issue. Additionally, after running for two hours, the fuzzers did not identify any new issues related to this function. 👍
Member
Author
|
Great! Thank you for testing it, Kleis. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
vips__token_get()could read a byte beyond the end of strings containing unterminated quotes.See #4104
I'll expose this function in pyvips as well so we can add some tests, it's clearly tricker to get right than it looks.