Skip to content
New issue

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

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

Already on GitHub? # to your account

Add bounds check to images loaded from bufferviews #512

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

ctrlaltf2
Copy link
Contributor

@ctrlaltf2 ctrlaltf2 commented Jan 21, 2025

Describe the issue

glTF binary files containing embedded images can read outside of bounds of their bufferview and lead to a SIGSEGV/general memory read out of bounds; discovered with AFL++.

To Reproduce

Load the GLB from carbonFibre.zip to a tinygltf::TinyGLTF::LoadBinaryFromMemory call and have ASAN on. The callstack is a bit misleading since it's downstream of what is actually introducing the issue:

   #0 0x419622 in stbi__get8 stb_image_1_30.h:1617
   #1 0x423885 in stbi__get_marker stb_image_1_30.h:2923
   #2 0x4333d1 in stbi__decode_jpeg_header stb_image_1_30.h:3371
   #3 0x451e36 in stbi__jpeg_info_raw stb_image_1_30.h:4058
   #4 0x451e36 in stbi__jpeg_info stb_image_1_30.h:4075
   #5 0x451e36 in stbi__info_main stb_image_1_30.h:7635
   #6 0x456176 in stbi_info_from_memory stb_image_1_30.h:7738
   #7 0x46f2ad in tinygltf::LoadImageData(tinygltf::Image*, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, int, int, unsigned char const*, int, void*) tiny_gltf.h:2630
   #8 0x4ec3d0 in std::function<bool (tinygltf::Image*, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, int, int, unsigned char const*, int, void*)>::operator()(tinygltf::Image*, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, int, int, unsigned char const*, int, void*) const /nix/store/l89iqc7am6i60y8vk507zwrzxf0wcd3v-gcc-14-20241116/include/c++/14-20241116/bits/std_function.h:591
   #9 0x4ec3d0 in operator() tiny_gltf.h:6463
   #10 0x4ef785 in ForEachInArray<tinygltf::TinyGLTF::LoadFromString(tinygltf::Model*, std::string*, std::string*, char const*, unsigned int, const std::string&, unsigned int)::<lambda(const nlohmann::json&)> > tiny_gltf.h:5981
   #11 0x4f2c9b in tinygltf::TinyGLTF::LoadFromString(tinygltf::Model*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, char const*, unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int) tiny_gltf.h:6417
   #12 0x500a59 in tinygltf::TinyGLTF::LoadBinaryFromMemory(tinygltf::Model*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, unsigned char const*, unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int) tiny_gltf.h:6918

With patch

Instead of a SIGSEGV, the read is caught and the err parameter is filled out (in this testcase) with image[0] bufferView "3" indexed out of bounds of its buffer. noting what went wrong.

@syoyo
Copy link
Owner

syoyo commented Jan 22, 2025

Good find!

Confirmed the issue with

$ clang++  -fsanitize=address  -std=c++11 -g -O0 -o loader_example loader_example.cc
$ ./loader_example carbonFibre.glb

Reading binary glTF
AddressSanitizer:DEADLYSIGNAL
=================================================================
==500549==ERROR: AddressSanitizer: SEGV on unknown address 0x62f000024b20 (pc 0x55a1b7c039ca bp 0x7fff9fd57f20 sp 0x7fff9fd57eb0 T0)
==500549==The signal is caused by a READ memory access.
    #0 0x55a1b7c039ca in stbi__get8(stbi__context*) /home/syoyo/work/tinygltf/././stb_image.h:1232:14
    #1 0x55a1b7c00afa in stbi__get_marker(stbi__jpeg*) /home/syoyo/work/tinygltf/././stb_image.h:2425:8
    #2 0x55a1b7bfe34d in stbi__decode_jpeg_header(stbi__jpeg*, int) /home/syoyo/work/tinygltf/././stb_image.h:2799:8
    #3 0x55a1b7bf762a in stbi__jpeg_test(stbi__context*) /home/syoyo/work/tinygltf/././stb_image.h:3425:8
    #4 0x55a1b7bf71c7 in stbi__load_main(stbi__context*, int*, int*, int*, int) /home/syoyo/work/tinygltf/././stb_image.h:943:8
    #5 0x55a1b7bad3e7 in stbi__load_flip(stbi__context*, int*, int*, int*, int) /home/syoyo/work/tinygltf/././stb_image.h:982:28
    #6 0x55a1b7bad8b1 in stbi_load_from_memory /home/syoyo/work/tinygltf/././stb_image.h:1071:11
    #7 0x55a1b7bc0880 in tinygltf::LoadImageData(tinygltf::Image*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, int, int, unsigned char const*, int, void*) /home/syoyo/work/tinygltf/./tiny_gltf.h:1339:25
    #8 0x55a1b7bcc744 in tinygltf::TinyGLTF::LoadFromString(tinygltf::Model*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, char const*, unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, unsigned int) /home/syoyo/work/tinygltf/./tiny_gltf.h:3362:22
    #9 0x55a1b7be2538 in tinygltf::TinyGLTF::LoadBinaryFromMemory(tinygltf::Model*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, unsigned char const*, unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, unsigned int) /home/syoyo/work/tinygltf/./tiny_gltf.h:3663:14
    #10 0x55a1b7be2e5c in tinygltf::TinyGLTF::LoadBinaryFromFile(tinygltf::Model*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, unsigned int) /home/syoyo/work/tinygltf/./tiny_gltf.h:3702:14
    #11 0x55a1b7bf0e9e in main /home/syoyo/work/tinygltf/loader_example.cc:613:20
    #12 0x7f0796258082 in __libc_start_main /build/glibc-LcI20x/glibc-2.31/csu/../csu/libc-start.c:308:16
    #13 0x55a1b7aebbfd in _start (/home/syoyo/work/tinygltf/loader_example+0x2ebfd) (BuildId: 49d846a29e2431e12c6ff5a1a485406e969429bd)

The PR will be merged soon.

@syoyo syoyo added the bug label Jan 22, 2025
@syoyo syoyo merged commit a5e653e into syoyo:release Jan 22, 2025
8 checks passed
@syoyo
Copy link
Owner

syoyo commented Jan 22, 2025

Merged! Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants