Incorrect size check in ReadWholeFile

When fuzzing tinygltf, I found a crash of tinygltf trying to allocate a chunk of 0x7fffffffffffffff.

POC:

{"images":[{"uri":"%!QAAAQAAA5"}],"asset":{"version":""}}

Running ./fuzz_gltf poc results in:

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 822025005
INFO: Loaded 1 modules   (13727 inline 8-bit counters): 13727 [0x55555598ab90, 0x55555598e12f), 
INFO: Loaded 1 PC tables (13727 PCs): 13727 [0x55555598e130,0x5555559c3b20), 
./fuzz_gltf: Running 1 inputs 1 time(s) each.
Running: poc
=================================================================
==49930==ERROR: AddressSanitizer: requested allocation size 0x7fffffffffffffff (0x8000000000001000 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)
    #0 0x5555556cc3fd in operator new(unsigned long) (/tmp/out/tiny2/tests/fuzzer/build/fuzz_gltf+0x1783fd) (BuildId: ed34c3e6c28025f07f69248a3ced4d442acd605a)
    #1 0x555555607680 in std::vector<unsigned char, std::allocator<unsigned char>>::_M_default_append(unsigned long) (/tmp/out/tiny2/tests/fuzzer/build/fuzz_gltf+0xb3680) (BuildId: ed34c3e6c28025f07f69248a3ced4d442acd605a)
    #2 0x5555556f87ec in std::vector<unsigned char, std::allocator<unsigned char>>::resize(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:940:4
    #3 0x5555556fbee3 in tinygltf::ReadWholeFile(std::vector<unsigned char, std::allocator<unsigned 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&, void*) /tmp/out/tiny2/tests/fuzzer/build/../../../tiny_gltf.h:2855:8
    #4 0x55555586890e in tinygltf::LoadExternalFile(std::vector<unsigned char, std::allocator<unsigned 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>>*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, bool, unsigned long, bool, tinygltf::FsCallbacks*) /tmp/out/tiny2/tests/fuzzer/build/../../../tiny_gltf.h:2390:7

It seems that this line return 0x7fffffffffffffff:

tinygltf/tiny_gltf.h

Line 2839 in 5c06b7d

size_t sz = static_cast<size_t>(f.tellg());

Not sure whether this is a bug.

@Changochen Thanks! Its a curious situation...

I guess you are using Linux. I can reproduce the issue with Ubuntu + clang + gnustl.

I have added some debug printf to tiny_gltf.h in my local repo.

Firstly, uri is decoded as AAAQAAA5, then FindFile finds a file as ./AAAQAAA5(fopen in FileExists

tinygltf/tiny_gltf.h

Line 2676 in 5c06b7d

bool FileExists(const std::string &abs_filename, void *) {

does not report an error when opening un-existing AAAQAAA5 file), then ifstream reports its file can be opened correctly(even such a file does not exist)

filename AAAQAAA5
filepath ./AAAQAAA5
filepath ./AAAQAAA5
f.good 1
f.eof 0
f.fail 0
f.bad 0
tell = 9223372036854775807
sz = 9223372036854775807

Probably we also need a size check to tellg() reported value if it is an invalid value(0x7fffffffffffffff = int64_t max)

Another thing we need to investigate is:
When using "uri":"QAAAQAAA5", it works(file not found).
Builtin URI decoder

tinygltf/tiny_gltf.h

Line 2326 in 5c06b7d

static const std::string urldecode(const std::string &str) {

may add non-ASCII character to the decoded uri string for %! input

Hi @syoyo, thanks for the reply.

I did a bit debugging and found something interesting. Actually, the log you print in FileExists: filepath ./AAAQAAA5， the file name is ./\x00AAAQAAA5. However, this whole string (including the null char) belongs to the abs_filename, so if you use std::cout << abs_filename, it will print as ./AAAQAAA5. If you use printf instead, you will only see ./.

So the fopen is actually trying to open ./, which exists, so it return true. Then later the ifsteam is also trying to open ./ (with c_str).

So I think maybe %! causes the first character of the filename is overwritten to \x00.

BTW, I found that the file path is without any constraints. For example, {"images":[{"uri":"/etc/issue"}],"asset":{"version":""}} will open and read /etc/issue. Is this intended? Otherwise it might cause security problem if the data is used (like saving to an output file).

I did a bit debugging and found something interesting. Actually, the log you print in FileExists: filepath ./AAAQAAA5， the file name is ./\x00AAAQAAA5. However, this whole string (including the null char) belongs to the abs_filename, so if you use std::cout << abs_filename, it will print as ./AAAQAAA5. If you use printf instead, you will only see ./.

So the fopen is actually trying to open ./, which exists, so it return true. Then later the ifsteam is also trying to open ./ (with c_str).

So I think maybe %! causes the first character of the filename is overwritten to \x00.

@Changochen Thanks! So the fix will be

Check if a filename contains null-character(\x00) in the middle of string.
Check if open'ed file is a directory(fopen/ifstream successes, but tellg would return invalid size)

BTW, I found that the file path is without any constraints. For example, {"images":[{"uri":"/etc/issue"}],"asset":{"version":""}} will open and read /etc/issue. Is this intended? Otherwise it might cause security problem if the data is used (like saving to an output file).

Yes, it should be okay. If you need to consider security, you need to provide your own filesystem handler, or run an app in sandboxed environment(docker, wasm/wasi, runc, ...).

@Changochen Now TinyGLTF returns error for a filename containing null-character. Also add extra checks if a file can be read correctly. Also, I have added a feature to limit file size of external resources(images, buffers)

tinygltf/tiny_gltf.h

Line 1488 in 59cc44a

///

Thank you for reporting the issue!

@syoyo Thanks for the quick fix!