From ecfd37dee2ec212d6b385686a93ba9f5b1eba27e Mon Sep 17 00:00:00 2001 From: Syoyo Fujita Date: Sun, 23 Apr 2023 21:31:30 +0900 Subject: [PATCH 1/4] - Add GetFileSizeInBytes Filesystem Callback - Add feature to limit file size for external resources(images, buffers) - Use strlen to correctly retrieve a string from a string which contains multiple null-characters. - Return fail when opening a directory(Posix only). Fixes #416 --- tests/tester.cc | 17 ++++ tiny_gltf.h | 217 ++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 220 insertions(+), 14 deletions(-) diff --git a/tests/tester.cc b/tests/tester.cc index dc653ff..6d93d28 100644 --- a/tests/tester.cc +++ b/tests/tester.cc @@ -721,3 +721,20 @@ TEST_CASE("serialize-image-failure", "[issue-394]") { REQUIRE(false == result); REQUIRE(os.str().size() == 0); } + +TEST_CASE("filesize-check", "[issue-416]") { + + tinygltf::Model model; + tinygltf::TinyGLTF ctx; + std::string err; + std::string warn; + + ctx.SetMaxExternalFileSize(10); // 10 bytes. will fail to load texture image. + + bool ret = ctx.LoadASCIIFromFile(&model, &err, &warn, "../models/Cube/Cube.gltf"); + if (!err.empty()) { + std::cerr << err << std::endl; + } + + REQUIRE(false == ret); +} diff --git a/tiny_gltf.h b/tiny_gltf.h index e8f6f60..155dc79 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -1301,6 +1301,12 @@ typedef bool (*WriteWholeFileFunction)(std::string *, const std::string &, const std::vector &, void *); +/// +/// GetFileSizeFunction type. Signature for custom filesystem callbacks. +/// +typedef bool (*GetFileSizeFunction)(size_t *filesize_out, std::string *err, const std::string &abs_filename, + void *userdata); + /// /// A structure containing all required filesystem callbacks and a pointer to /// their user data. @@ -1310,6 +1316,7 @@ struct FsCallbacks { ExpandFilePathFunction ExpandFilePath; ReadWholeFileFunction ReadWholeFile; WriteWholeFileFunction WriteWholeFile; + GetFileSizeFunction GetFileSizeInBytes; // To avoid GetFileSize Win32 API, add `InBytes` suffix. void *user_data; // An argument that is passed to all fs callbacks }; @@ -1333,6 +1340,9 @@ bool ReadWholeFile(std::vector *out, std::string *err, bool WriteWholeFile(std::string *err, const std::string &filepath, const std::vector &contents, void *); + +bool GetFileSizeInBytes(size_t *filesize_out, std::string *err, const std::string &filepath, + void *); #endif /// @@ -1472,6 +1482,19 @@ class TinyGLTF { preserve_image_channels_ = onoff; } + /// + /// Set maximum allowed external file size in bytes. + /// Default: 2GB + /// Only effective for built-in ReadWholeFileFunction FS function. + /// + void SetMaxExternalFileSize(size_t max_bytes) { + max_external_file_size_ = max_bytes; + } + + size_t GetMaxExternalFileSize() const { + return max_external_file_size_; + } + bool GetPreserveImageChannels() const { return preserve_image_channels_; } private: @@ -1496,6 +1519,8 @@ class TinyGLTF { bool preserve_image_channels_ = false; /// Default false(expand channels to /// RGBA) for backward compatibility. + size_t max_external_file_size_{(std::numeric_limits::max)()}; // Default 2GB + // Warning & error messages std::string warn_; std::string err_; @@ -1503,11 +1528,11 @@ class TinyGLTF { FsCallbacks fs = { #ifndef TINYGLTF_NO_FS &tinygltf::FileExists, &tinygltf::ExpandFilePath, - &tinygltf::ReadWholeFile, &tinygltf::WriteWholeFile, + &tinygltf::ReadWholeFile, &tinygltf::WriteWholeFile, &tinygltf::GetFileSizeInBytes, nullptr // Fs callback user data #else - nullptr, nullptr, nullptr, nullptr, + nullptr, nullptr, nullptr, nullptr, nullptr, nullptr // Fs callback user data #endif @@ -1554,6 +1579,7 @@ class TinyGLTF { #ifndef TINYGLTF_NO_FS #include #include +#include // for is_directory check #endif #include @@ -2107,9 +2133,19 @@ static std::string FindFile(const std::vector &paths, return std::string(); } + // https://github.com/syoyo/tinygltf/issues/416 + // Use strlen() since std::string's size/length reports the number of elements in the buffer, not the length of string(null-terminated) + // strip null-character in the middle of string. + size_t slength = strlen(filepath.c_str()); + if (slength == 0) { + return std::string(); + } + + std::string cleaned_filepath = std::string(filepath.c_str()); + for (size_t i = 0; i < paths.size(); i++) { std::string absPath = - fs->ExpandFilePath(JoinPath(paths[i], filepath), fs->user_data); + fs->ExpandFilePath(JoinPath(paths[i], cleaned_filepath), fs->user_data); if (fs->FileExists(absPath, fs->user_data)) { return absPath; } @@ -2355,7 +2391,7 @@ bool URIDecode(const std::string &in_uri, std::string *out_uri, static bool LoadExternalFile(std::vector *out, std::string *err, std::string *warn, const std::string &filename, const std::string &basedir, bool required, - size_t reqBytes, bool checkSize, FsCallbacks *fs) { + size_t reqBytes, bool checkSize, size_t maxFileSize, FsCallbacks *fs) { if (fs == nullptr || fs->FileExists == nullptr || fs->ExpandFilePath == nullptr || fs->ReadWholeFile == nullptr) { // This is a developer error, assert() ? @@ -2381,6 +2417,29 @@ static bool LoadExternalFile(std::vector *out, std::string *err, return false; } + // Check file size + if (fs->GetFileSizeInBytes) { + + size_t file_size{0}; + std::string _err; + bool ok = fs-GetFileSizeInBytes(&file_size, &_err, filepath, fs->user_data); + if (!ok) { + if (_err.size()) { + if (failMsgOut) { + (*failMsgOut) += "Getting file size failed : " + filename + ", err = " + _err + "\n"; + } + } + return false; + } + + if (file_size > maxFileSize) { + if (failMsgOut) { + (*failMsgOut) += "File size " + std::to_string(file_size) + " exceeds maximum allowed file size " + std::to_string(maxFileSize) + " : " + filepath + "\n"; + } + return false; + } + } + std::vector buf; std::string fileReadErr; bool fileRead = @@ -2687,12 +2746,24 @@ bool FileExists(const std::string &abs_filename, void *) { #else #ifdef _WIN32 #if defined(_MSC_VER) || defined(__GLIBCXX__) || defined(_LIBCPP_VERSION) + + // First check if a file is a directory. + DWORD result = GetFileAttributesW(UTF8ToWchar(abs_filename).c_str()); + if (result == INVALID_FILE_ATTRIBUTES) { + return false; + } + if (result & FILE_ATTRIBUTE_DIRECTORY) { + return false; + } + } + FILE *fp = nullptr; errno_t err = _wfopen_s(&fp, UTF8ToWchar(abs_filename).c_str(), L"rb"); if (err != 0) { return false; } #else + // TODO: is_directory check FILE *fp = nullptr; errno_t err = fopen_s(&fp, abs_filename.c_str(), "rb"); if (err != 0) { @@ -2701,6 +2772,14 @@ bool FileExists(const std::string &abs_filename, void *) { #endif #else + struct stat sb; + if (stat(abs_filename.c_str(), &sb)) { + return false; + } + if (S_ISDIR(sb.st_mode)) { + return false; + } + FILE *fp = fopen(abs_filename.c_str(), "rb"); #endif if (fp) { @@ -2777,6 +2856,100 @@ std::string ExpandFilePath(const std::string &filepath, void *) { #endif } +bool GetFileSizeInBytes(size_t *filesize_out, std::string *err, + const std::string &filepath, void *userdata) { + (void)userdata; + +#ifdef TINYGLTF_ANDROID_LOAD_FROM_ASSETS + if (asset_manager) { + AAsset *asset = AAssetManager_open(asset_manager, filepath.c_str(), + AASSET_MODE_STREAMING); + if (!asset) { + if (err) { + (*err) += "File open error : " + filepath + "\n"; + } + return false; + } + size_t size = AAsset_getLength(asset); + + if (size == 0) { + if (err) { + (*err) += "Invalid file size : " + filepath + + " (does the path point to a directory?)"; + } + return false; + } + + return true; + } else { + if (err) { + (*err) += "No asset manager specified : " + filepath + "\n"; + } + return false; + } +#else +#ifdef _WIN32 +#if defined(__GLIBCXX__) // mingw + int file_descriptor = + _wopen(UTF8ToWchar(filepath).c_str(), _O_RDONLY | _O_BINARY); + __gnu_cxx::stdio_filebuf wfile_buf(file_descriptor, std::ios_base::in); + std::istream f(&wfile_buf); +#elif defined(_MSC_VER) || defined(_LIBCPP_VERSION) + // For libcxx, assume _LIBCPP_HAS_OPEN_WITH_WCHAR is defined to accept + // `wchar_t *` + std::ifstream f(UTF8ToWchar(filepath).c_str(), std::ifstream::binary); +#else + // Unknown compiler/runtime + std::ifstream f(filepath.c_str(), std::ifstream::binary); +#endif +#else + std::ifstream f(filepath.c_str(), std::ifstream::binary); +#endif + if (!f) { + if (err) { + (*err) += "File open error : " + filepath + "\n"; + } + return false; + } + + // For directory(and pipe?), peek() will fail(Posix gnustl/libc++ only) + int buf = f.peek(); + if (!f) { + if (err) { + (*err) += "File read error. Maybe empty file or invalid file : " + filepath + "\n"; + } + return false; + } + + f.seekg(0, f.end); + size_t sz = static_cast(f.tellg()); + + //std::cout << "sz = " << sz << "\n"; + f.seekg(0, f.beg); + + if (int64_t(sz) < 0) { + if (err) { + (*err) += "Invalid file size : " + filepath + + " (does the path point to a directory?)"; + } + return false; + } else if (sz == 0) { + if (err) { + (*err) += "File is empty : " + filepath + "\n"; + } + return false; + } else if (sz >= (std::numeric_limits::max)()) { + if (err) { + (*err) += "Invalid file size : " + filepath + "\n"; + } + return false; + } + + (*filesize_out) = sz; + return true; +#endif +} + bool ReadWholeFile(std::vector *out, std::string *err, const std::string &filepath, void *) { #ifdef TINYGLTF_ANDROID_LOAD_FROM_ASSETS @@ -2832,8 +3005,19 @@ bool ReadWholeFile(std::vector *out, std::string *err, return false; } + // For directory(and pipe?), peek() will fail(Posix gnustl/libc++ only) + int buf = f.peek(); + if (!f) { + if (err) { + (*err) += "File read error. Maybe empty file or invalid file : " + filepath + "\n"; + } + return false; + } + f.seekg(0, f.end); size_t sz = static_cast(f.tellg()); + + //std::cout << "sz = " << sz << "\n"; f.seekg(0, f.beg); if (int64_t(sz) < 0) { @@ -2847,6 +3031,11 @@ bool ReadWholeFile(std::vector *out, std::string *err, (*err) += "File is empty : " + filepath + "\n"; } return false; + } else if (sz >= (std::numeric_limits::max)()) { + if (err) { + (*err) += "Invalid file size : " + filepath + "\n"; + } + return false; } out->resize(sz); @@ -3873,7 +4062,7 @@ static bool ParseAsset(Asset *asset, std::string *err, const detail::json &o, static bool ParseImage(Image *image, const int image_idx, std::string *err, std::string *warn, const detail::json &o, bool store_original_json_for_extras_and_extensions, - const std::string &basedir, FsCallbacks *fs, + const std::string &basedir, const size_t max_file_size, FsCallbacks *fs, const URICallbacks *uri_cb, LoadImageDataFunction *LoadImageData = nullptr, void *load_image_user_data = nullptr) { @@ -3973,8 +4162,8 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err, if (!DecodeDataURI(&img, image->mimeType, uri, 0, false)) { if (err) { (*err) += "Failed to decode 'uri' for image[" + - std::to_string(image_idx) + "] name = [" + image->name + - "]\n"; + std::to_string(image_idx) + "] name = \"" + image->name + + "\"\n"; } return false; } @@ -3999,10 +4188,10 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err, if (!LoadExternalFile(&img, err, warn, decoded_uri, basedir, /* required */ false, /* required bytes */ 0, - /* checksize */ false, fs)) { + /* checksize */ false, /* max file size */ max_file_size, fs)) { if (warn) { (*warn) += "Failed to load external 'uri' for image[" + - std::to_string(image_idx) + "] name = [" + image->name + + std::to_string(image_idx) + "] name = [" + decoded_uri + "]\n"; } // If the image cannot be loaded, keep uri as image->uri. @@ -4176,7 +4365,7 @@ static bool ParseOcclusionTextureInfo( static bool ParseBuffer(Buffer *buffer, std::string *err, const detail::json &o, bool store_original_json_for_extras_and_extensions, FsCallbacks *fs, const URICallbacks *uri_cb, - const std::string &basedir, bool is_binary = false, + const std::string &basedir, const size_t max_buffer_size, bool is_binary = false, const unsigned char *bin_data = nullptr, size_t bin_size = 0) { size_t byteLength; @@ -4228,7 +4417,7 @@ static bool ParseBuffer(Buffer *buffer, std::string *err, const detail::json &o, } if (!LoadExternalFile(&buffer->data, err, /* warn */ nullptr, decoded_uri, basedir, /* required */ true, - byteLength, /* checkSize */ true, fs)) { + byteLength, /* checkSize */ true, /* max_file_size */max_buffer_size, fs)) { return false; } } @@ -4276,7 +4465,7 @@ static bool ParseBuffer(Buffer *buffer, std::string *err, const detail::json &o, } if (!LoadExternalFile(&buffer->data, err, /* warn */ nullptr, decoded_uri, basedir, /* required */ true, byteLength, - /* checkSize */ true, fs)) { + /* checkSize */ true, /* max file size */max_buffer_size, fs)) { return false; } } @@ -5811,7 +6000,7 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn, Buffer buffer; if (!ParseBuffer(&buffer, err, o, store_original_json_for_extras_and_extensions_, &fs, - &uri_cb, base_dir, is_binary_, bin_data_, bin_size_)) { + &uri_cb, base_dir, max_external_file_size_, is_binary_, bin_data_, bin_size_)) { return false; } @@ -6082,7 +6271,7 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn, Image image; if (!ParseImage(&image, idx, err, warn, o, store_original_json_for_extras_and_extensions_, base_dir, - &fs, &uri_cb, &this->LoadImageData, + max_external_file_size_, &fs, &uri_cb, &this->LoadImageData, load_image_user_data)) { return false; } From b534b6b0d8d1540e035cf0c4af4a54abfd5b62bb Mon Sep 17 00:00:00 2001 From: Syoyo Fujita Date: Sun, 23 Apr 2023 21:40:23 +0900 Subject: [PATCH 2/4] Fix syntax. --- tiny_gltf.h | 1 - 1 file changed, 1 deletion(-) diff --git a/tiny_gltf.h b/tiny_gltf.h index 155dc79..4f2f079 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -2755,7 +2755,6 @@ bool FileExists(const std::string &abs_filename, void *) { if (result & FILE_ATTRIBUTE_DIRECTORY) { return false; } - } FILE *fp = nullptr; errno_t err = _wfopen_s(&fp, UTF8ToWchar(abs_filename).c_str(), L"rb"); From 877d856e717a26a2b5e956d1703ad63625708b9b Mon Sep 17 00:00:00 2001 From: Syoyo Fujita Date: Sun, 23 Apr 2023 21:47:31 +0900 Subject: [PATCH 3/4] Format error message. Add regression test of issue-416. --- tests/issue-416.gltf | 1 + tests/tester.cc | 19 +++++++++++++++++++ tiny_gltf.h | 12 ++++++------ 3 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 tests/issue-416.gltf diff --git a/tests/issue-416.gltf b/tests/issue-416.gltf new file mode 100644 index 0000000..f1244ab --- /dev/null +++ b/tests/issue-416.gltf @@ -0,0 +1 @@ +{"images":[{"uri":"%!QAAAQAAA5"}],"asset":{"version":""}} diff --git a/tests/tester.cc b/tests/tester.cc index 6d93d28..8ebeeae 100644 --- a/tests/tester.cc +++ b/tests/tester.cc @@ -738,3 +738,22 @@ TEST_CASE("filesize-check", "[issue-416]") { REQUIRE(false == ret); } + +TEST_CASE("load-issue-416-model", "[issue-416]") { + + tinygltf::Model model; + tinygltf::TinyGLTF ctx; + std::string err; + std::string warn; + + bool ret = ctx.LoadASCIIFromFile(&model, &err, &warn, "issue-416.gltf"); + if (!warn.empty()) { + std::cout << "WARN:" << warn << std::endl; + } + if (!err.empty()) { + std::cerr << "ERR:" << err << std::endl; + } + + // external file load fails, but reading glTF itself is ok. + REQUIRE(true == ret); +} diff --git a/tiny_gltf.h b/tiny_gltf.h index 4f2f079..9e1351b 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -4177,8 +4177,8 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err, if (!uri_cb->decode(uri, &decoded_uri, uri_cb->user_data)) { if (warn) { (*warn) += "Failed to decode 'uri' for image[" + - std::to_string(image_idx) + "] name = [" + image->name + - "]\n"; + std::to_string(image_idx) + "] name = \"" + image->name + + "\"\n"; } // Image loading failure is not critical to overall gltf loading. @@ -4190,8 +4190,8 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err, /* checksize */ false, /* max file size */ max_file_size, fs)) { if (warn) { (*warn) += "Failed to load external 'uri' for image[" + - std::to_string(image_idx) + "] name = [" + decoded_uri + - "]\n"; + std::to_string(image_idx) + "] name = \"" + decoded_uri + + "\"\n"; } // If the image cannot be loaded, keep uri as image->uri. return true; @@ -4200,8 +4200,8 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err, if (img.empty()) { if (warn) { (*warn) += "Image data is empty for image[" + - std::to_string(image_idx) + "] name = [" + image->name + - "] \n"; + std::to_string(image_idx) + "] name = \"" + image->name + + "\" \n"; } return false; } From 1a5046e06bce08f560c0303a9a0b0dd07b50f5fb Mon Sep 17 00:00:00 2001 From: Syoyo Fujita Date: Sun, 23 Apr 2023 23:08:41 +0900 Subject: [PATCH 4/4] Fix MSVC compile failure on AppVeyor CI. --- tiny_gltf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tiny_gltf.h b/tiny_gltf.h index 9e1351b..1e4bb3c 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -1519,7 +1519,7 @@ class TinyGLTF { bool preserve_image_channels_ = false; /// Default false(expand channels to /// RGBA) for backward compatibility. - size_t max_external_file_size_{(std::numeric_limits::max)()}; // Default 2GB + size_t max_external_file_size_{size_t((std::numeric_limits::max)())}; // Default 2GB // Warning & error messages std::string warn_;