diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..ed5ebfd --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,34 @@ +# Repository Guidelines + +## Project Structure & Module Organization +- Core library lives in `tiny_gltf.h` (header-only) with `tiny_gltf.cc` provided for the amalgamated implementation flags. Keep public API updates localized and documented. +- Example viewers and utilities sit under `examples/`; use them as references for loading, validation, and WASM builds. Temporary build outputs belong in `build/` (git-ignored). +- Tests reside in `tests/` with sample assets in `data/` and `models/`; avoid committing generated binaries in `build/`, `tmp/`, or `tests/tester*`. + +## Build, Test, and Development Commands +- Quick build of the loader example: `make` (uses clang++, C++11, optional `EXTRA_CXXFLAGS` for sanitizers). +- Unit tests: `cd tests && make && ./tester && ./tester_noexcept`. +- Parsing regression run: build `loader_example`, then `python test_runner.py` (requires local glTF-Sample-Models checkout and path update inside the script). +- CMake alternative: `cmake -S . -B build && cmake --build build` for IDE integration or non-clang toolchains. +- Lint header: `python deps/cpplint.py tiny_gltf.h`. + +## Coding Style & Naming Conventions +- C++11, two-space indent, braces on the same line; mirror existing spacing and comment style in `tiny_gltf.h`. +- Prefer `std::` facilities and minimal dependencies; keep new symbols in the `tinygltf` namespace. +- Public API names stay PascalCase for types and camelCase for functions; keep enums/macros consistent with existing `TINYGLTF_*` patterns. +- Guard optional features with the established `TINYGLTF_*` defines; avoid introducing new globals without discussion. + +## Testing Guidelines +- Framework: Catch2 single-header (`tests/catch.hpp`); add `TEST_CASE` blocks alongside related helpers in `tests/tester.cc`. +- Provide coverage for both exception-enabled and `TINYGLTF_NOEXCEPTION` builds; run both `tester` binaries before submitting. +- For new formats or parsing code, add assets under `tests/` or reference `data/` and note provenance. + +## Commit & Pull Request Guidelines +- Commit messages: concise, present-tense imperatives mirroring existing history (e.g., “Add bounds check to images loaded from bufferviews”). +- PRs should describe the change, motivation, and testing (`tester`, `tester_noexcept`, fuzzing if relevant); link related issues. +- Include platform notes if behavior differs (Windows vs. POSIX, filesystem callbacks, WASM). Add before/after metrics when touching performance-sensitive paths. + +## Security & Configuration Tips +- Handle external data defensively: validate buffer sizes, offsets, and URI handling; prefer bounded allocations. +- Keep optional callbacks (`fs::`, URI, image) robust against untrusted input; document new failure modes. +- Avoid committing sample assets with unclear licensing; reuse existing test fixtures where possible. diff --git a/tests/tester.cc b/tests/tester.cc index b06c30d..c3d1944 100644 --- a/tests/tester.cc +++ b/tests/tester.cc @@ -758,6 +758,194 @@ TEST_CASE("load-issue-416-model", "[issue-416]") { REQUIRE(true == ret); } +TEST_CASE("reject-unsafe-paths", "[security]") { + tinygltf::TinyGLTF ctx; + + SECTION("parent-directory reference is rejected") { + tinygltf::Model model; + std::string err; + std::string warn; + + const std::string gltf = R"({ + "asset": {"version": "2.0"}, + "buffers": [ + {"uri": "../secret.bin", "byteLength": 4} + ] + })"; + + bool ret = ctx.LoadASCIIFromString(&model, &err, &warn, gltf.c_str(), + static_cast(gltf.size()), + "."); + REQUIRE_FALSE(ret); + REQUIRE_THAT(err, Catch::Contains("Rejected unsafe filename")); + } + + SECTION("absolute path is rejected") { + tinygltf::Model model; + std::string err; + std::string warn; + + const std::string gltf = R"({ + "asset": {"version": "2.0"}, + "buffers": [ + {"uri": "/tmp/secret.bin", "byteLength": 4} + ] + })"; + + bool ret = ctx.LoadASCIIFromString(&model, &err, &warn, gltf.c_str(), + static_cast(gltf.size()), + "."); + REQUIRE_FALSE(ret); + REQUIRE_THAT(err, Catch::Contains("Rejected unsafe filename")); + } +} + +TEST_CASE("data-uri-size-limit", "[security]") { + tinygltf::TinyGLTF ctx; + ctx.SetMaxDataURISize(16); // small limit to exercise rejection path. + + const std::string payload(24, 'A'); // decodes to 18 bytes. + const std::string data_uri = + "data:application/octet-stream;base64," + payload; + const std::string gltf = R"({ + "asset": {"version": "2.0"}, + "buffers": [ + {"uri": ")" + data_uri + R"(", "byteLength": 18} + ] + })"; + + tinygltf::Model model; + std::string err; + std::string warn; + + bool ret = ctx.LoadASCIIFromString(&model, &err, &warn, gltf.c_str(), + static_cast(gltf.size()), + "."); + REQUIRE_FALSE(ret); + REQUIRE_THAT(err, + Catch::Contains("Data URI for buffer exceeds maximum allowed")); + + SECTION("ceiling estimation rejects near-limit payloads") { + tinygltf::TinyGLTF ctx2; + ctx2.SetMaxDataURISize(4); + // 8 chars base64 -> 6 decoded bytes, should be rejected by max size. + const std::string small_payload(8, 'A'); + const std::string small_uri = + "data:application/octet-stream;base64," + small_payload; + const std::string gltf_small = R"({ + "asset": {"version": "2.0"}, + "buffers": [ + {"uri": ")" + small_uri + R"(", "byteLength": 6} + ] + })"; + + tinygltf::Model m2; + std::string err2; + std::string warn2; + bool ok = ctx2.LoadASCIIFromString( + &m2, &err2, &warn2, gltf_small.c_str(), + static_cast(gltf_small.size()), "."); + REQUIRE_FALSE(ok); + REQUIRE_THAT(err2, + Catch::Contains("Data URI for buffer exceeds maximum allowed")); + } +} + +TEST_CASE("max-external-file-size", "[security]") { + tinygltf::TinyGLTF ctx; + ctx.SetMaxExternalFileSize(16); // small limit to force rejection. + + const std::string gltf_path = "oversize.gltf"; + { + std::ofstream ofs(gltf_path, std::ios::binary); + ofs << R"({"asset":{"version":"2.0"}})" << std::string(64, ' '); + } + + tinygltf::Model model; + std::string err; + std::string warn; + bool ok = ctx.LoadASCIIFromFile(&model, &err, &warn, gltf_path); + REQUIRE_FALSE(ok); + REQUIRE_THAT(err, + Catch::Contains("exceeds maximum allowed file size")); + std::remove(gltf_path.c_str()); + + const std::string glb_path = "oversize.glb"; + { + std::ofstream ofs(glb_path, std::ios::binary); + ofs << std::string(32, '\0'); + } + + err.clear(); + warn.clear(); + ok = ctx.LoadBinaryFromFile(&model, &err, &warn, glb_path); + REQUIRE_FALSE(ok); + REQUIRE_THAT(err, + Catch::Contains("exceeds maximum allowed file size")); + std::remove(glb_path.c_str()); +} + +TEST_CASE("max-size-in-memory", "[security]") { + tinygltf::TinyGLTF ctx; + ctx.SetMaxExternalFileSize(16); + + // LoadASCIIFromString should reject oversized input. + { + tinygltf::Model model; + std::string err; + std::string warn; + std::string large_json(20, ' '); + bool ok = ctx.LoadASCIIFromString( + &model, &err, &warn, large_json.c_str(), + static_cast(large_json.size()), "."); + REQUIRE_FALSE(ok); + REQUIRE_THAT(err, Catch::Contains("Input size exceeds maximum")); + } + + // LoadBinaryFromMemory should reject oversized input even before parsing. + { + tinygltf::Model model; + std::string err; + std::string warn; + std::array bytes{}; + // Craft minimal glTF header; size still exceeds limit and should be + // rejected early. + bytes[0] = 'g'; bytes[1] = 'l'; bytes[2] = 'T'; bytes[3] = 'F'; + bytes[4] = 2; // version little-endian + bool ok = ctx.LoadBinaryFromMemory(&model, &err, &warn, bytes.data(), + static_cast(bytes.size()), + "."); + REQUIRE_FALSE(ok); + REQUIRE_THAT(err, Catch::Contains("Input size exceeds maximum")); + } + + SECTION("GLB length mismatch is rejected in strict mode") { + tinygltf::TinyGLTF ctx_strict; + ctx_strict.SetParseStrictness(tinygltf::ParseStrictness::Strict); + + // Construct a minimal GLB with length field smaller than actual buffer. + std::array glb{}; + glb[0] = 'g'; glb[1] = 'l'; glb[2] = 'T'; glb[3] = 'F'; + glb[4] = 2; // version + uint32_t length = 24; // claimed length (smaller than actual buffer size) + memcpy(&glb[8], &length, 4); + uint32_t json_len = 4; + memcpy(&glb[12], &json_len, 4); + uint32_t json_fmt = 0x4E4F534A; // "JSON" + memcpy(&glb[16], &json_fmt, 4); + + tinygltf::Model model; + std::string err; + std::string warn; + bool ok = ctx_strict.LoadBinaryFromMemory( + &model, &err, &warn, glb.data(), + static_cast(glb.size()), "."); + REQUIRE_FALSE(ok); + REQUIRE_THAT(err, + Catch::Contains("Length field does not match data size")); + } +} + TEST_CASE("serialize-empty-node", "[issue-457]") { tinygltf::Model m; // Add default constructed node to model diff --git a/tiny_gltf.h b/tiny_gltf.h index 0eedd65..4526548 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -249,7 +249,10 @@ static inline int32_t GetNumComponentsInType(uint32_t ty) { // TODO(syoyo): Move these functions to TinyGLTF class bool IsDataURI(const std::string &in); bool DecodeDataURI(std::vector *out, std::string &mime_type, - const std::string &in, size_t reqBytes, bool checkSize); + const std::string &in, size_t reqBytes, bool checkSize, + size_t maxSize); + +static const size_t kDefaultMaxDataUriSize = size_t(100) * 1024 * 1024; // 100 MB #ifdef __clang__ #pragma clang diagnostic push @@ -1564,6 +1567,14 @@ class TinyGLTF { size_t GetMaxExternalFileSize() const { return max_external_file_size_; } + /// + /// Set maximum allowed decoded Data URI size in bytes. + /// Default: 100MB + /// + void SetMaxDataURISize(size_t max_bytes) { max_data_uri_size_ = max_bytes; } + + size_t GetMaxDataURISize() const { return max_data_uri_size_; } + private: /// /// Loads glTF asset from string(memory). @@ -1592,6 +1603,7 @@ class TinyGLTF { size_t max_external_file_size_{ size_t((std::numeric_limits::max)())}; // Default 2GB + size_t max_data_uri_size_{kDefaultMaxDataUriSize}; // Default 100MB // Warning & error messages std::string warn_; @@ -2228,6 +2240,40 @@ static std::string JoinPath(const std::string &path0, } } +// Reject absolute or parent-relative paths so external resources cannot escape +// the asset root. +static bool IsAbsolutePath(const std::string &path) { + if (path.empty()) { + return false; + } + if (path[0] == '/' || path[0] == '\\') { + return true; + } + if (path.size() > 1 && + ((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z')) && + path[1] == ':') { + return true; // Windows drive letter. + } + return false; +} + +static bool ContainsParentReference(const std::string &path) { + size_t start = 0; + while (start < path.size()) { + size_t end = path.find_first_of("/\\", start); + size_t len = (end == std::string::npos) ? std::string::npos : end - start; + std::string segment = path.substr(start, len); + if (segment == "..") { + return true; + } + if (end == std::string::npos) { + break; + } + start = end + 1; + } + return false; +} + static std::string FindFile(const std::vector &paths, const std::string &filepath, FsCallbacks *fs) { if (fs == nullptr || fs->ExpandFilePath == nullptr || @@ -2497,6 +2543,15 @@ static bool LoadExternalFile(std::vector *out, std::string *err, const std::string &basedir, bool required, size_t reqBytes, bool checkSize, size_t maxFileSize, FsCallbacks *fs) { + if (IsAbsolutePath(filename) || ContainsParentReference(filename)) { + if (required && err) { + (*err) += "Rejected unsafe filename: " + filename + "\n"; + } else if (!required && warn) { + (*warn) += "Rejected unsafe filename: " + filename + "\n"; + } + return false; + } + if (fs == nullptr || fs->FileExists == nullptr || fs->ExpandFilePath == nullptr || fs->ReadWholeFile == nullptr) { // This is a developer error, assert() ? @@ -2561,6 +2616,14 @@ static bool LoadExternalFile(std::vector *out, std::string *err, } size_t sz = buf.size(); + if (sz > maxFileSize) { + if (failMsgOut) { + (*failMsgOut) += "File size " + std::to_string(sz) + + " exceeds maximum allowed file size " + + std::to_string(maxFileSize) + " : " + filepath + "\n"; + } + return false; + } if (sz == 0) { if (failMsgOut) { (*failMsgOut) += "File is empty : " + filepath + "\n"; @@ -3376,58 +3439,57 @@ bool IsDataURI(const std::string &in) { } bool DecodeDataURI(std::vector *out, std::string &mime_type, - const std::string &in, size_t reqBytes, bool checkSize) { - std::string header = "data:application/octet-stream;base64,"; - std::string data; - if (in.find(header) == 0) { - data = base64_decode(in.substr(header.size())); // cut mime string. + const std::string &in, size_t reqBytes, bool checkSize, + size_t maxSize) { + auto exceeds_limit = [&](size_t payload_len) { + // Base64 inflates by roughly 4/3; use a ceiling bound to short-circuit + // obviously oversized payloads without decoding. + if (payload_len > (std::numeric_limits::max)() - 3) { + return true; + } + size_t estimated = ((payload_len + 3) / 4) * 3; + return (estimated > maxSize); + }; + + auto try_decode = [&](const std::string &header, + const char *mime) -> std::string { + if (in.find(header) != 0) { + return std::string(); + } + size_t payload_len = in.size() - header.size(); + if (exceeds_limit(payload_len)) { + return std::string(); + } + if (mime) { + mime_type = mime; + } + return base64_decode(in.substr(header.size())); // cut mime string. + }; + + std::string data = try_decode("data:application/octet-stream;base64,", nullptr); + + if (data.empty()) { + data = try_decode("data:image/jpeg;base64,", "image/jpeg"); } if (data.empty()) { - header = "data:image/jpeg;base64,"; - if (in.find(header) == 0) { - mime_type = "image/jpeg"; - data = base64_decode(in.substr(header.size())); // cut mime string. - } + data = try_decode("data:image/png;base64,", "image/png"); } if (data.empty()) { - header = "data:image/png;base64,"; - if (in.find(header) == 0) { - mime_type = "image/png"; - data = base64_decode(in.substr(header.size())); // cut mime string. - } + data = try_decode("data:image/bmp;base64,", "image/bmp"); } if (data.empty()) { - header = "data:image/bmp;base64,"; - if (in.find(header) == 0) { - mime_type = "image/bmp"; - data = base64_decode(in.substr(header.size())); // cut mime string. - } + data = try_decode("data:image/gif;base64,", "image/gif"); } if (data.empty()) { - header = "data:image/gif;base64,"; - if (in.find(header) == 0) { - mime_type = "image/gif"; - data = base64_decode(in.substr(header.size())); // cut mime string. - } + data = try_decode("data:text/plain;base64,", "text/plain"); } if (data.empty()) { - header = "data:text/plain;base64,"; - if (in.find(header) == 0) { - mime_type = "text/plain"; - data = base64_decode(in.substr(header.size())); - } - } - - if (data.empty()) { - header = "data:application/gltf-buffer;base64,"; - if (in.find(header) == 0) { - data = base64_decode(in.substr(header.size())); - } + data = try_decode("data:application/gltf-buffer;base64,", nullptr); } // TODO(syoyo): Allow empty buffer? #229 @@ -3435,6 +3497,10 @@ bool DecodeDataURI(std::vector *out, std::string &mime_type, return false; } + if (data.size() > maxSize) { + return false; + } + if (checkSize) { if (data.size() != reqBytes) { return false; @@ -4303,6 +4369,7 @@ 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, const size_t max_file_size, + const size_t max_data_uri_size, FsCallbacks *fs, const URICallbacks *uri_cb, const LoadImageDataFunction& LoadImageData = nullptr, void *load_image_user_data = nullptr) { @@ -4384,7 +4451,8 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err, std::vector img; if (IsDataURI(uri)) { - if (!DecodeDataURI(&img, image->mimeType, uri, 0, false)) { + if (!DecodeDataURI(&img, image->mimeType, uri, 0, false, + max_data_uri_size)) { if (err) { (*err) += "Failed to decode 'uri' for image[" + std::to_string(image_idx) + "] name = \"" + image->name + @@ -4534,7 +4602,8 @@ 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, - const size_t max_buffer_size, bool is_binary = false, + const size_t max_buffer_size, + const size_t max_data_uri_size, bool is_binary = false, const unsigned char *bin_data = nullptr, size_t bin_size = 0) { size_t byteLength; @@ -4569,9 +4638,16 @@ static bool ParseBuffer(Buffer *buffer, std::string *err, const detail::json &o, if (!buffer->uri.empty()) { // First try embedded data URI. if (IsDataURI(buffer->uri)) { + if (byteLength > max_data_uri_size) { + if (err) { + (*err) += "Data URI for buffer exceeds maximum allowed size (" + + std::to_string(max_data_uri_size) + " bytes).\n"; + } + return false; + } std::string mime_type; if (!DecodeDataURI(&buffer->data, mime_type, buffer->uri, byteLength, - true)) { + true, max_data_uri_size)) { if (err) { (*err) += "Failed to decode 'uri' : " + buffer->uri + " in Buffer\n"; @@ -4620,9 +4696,16 @@ static bool ParseBuffer(Buffer *buffer, std::string *err, const detail::json &o, } else { if (IsDataURI(buffer->uri)) { + if (byteLength > max_data_uri_size) { + if (err) { + (*err) += "Data URI for buffer exceeds maximum allowed size (" + + std::to_string(max_data_uri_size) + " bytes).\n"; + } + return false; + } std::string mime_type; if (!DecodeDataURI(&buffer->data, mime_type, buffer->uri, byteLength, - true)) { + true, max_data_uri_size)) { if (err) { (*err) += "Failed to decode 'uri' : " + buffer->uri + " in Buffer\n"; } @@ -6164,8 +6247,8 @@ 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, max_external_file_size_, is_binary_, - bin_data_, bin_size_)) { + &uri_cb, base_dir, max_external_file_size_, + max_data_uri_size_, is_binary_, bin_data_, bin_size_)) { return false; } @@ -6424,7 +6507,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, - max_external_file_size_, &fs, &uri_cb, + max_external_file_size_, max_data_uri_size_, &fs, &uri_cb, this->LoadImageData, load_image_user_data)) { return false; } @@ -6701,6 +6784,14 @@ bool TinyGLTF::LoadASCIIFromString(Model *model, std::string *err, unsigned int length, const std::string &base_dir, unsigned int check_sections) { + if (length > max_external_file_size_) { + if (err) { + (*err) = "Input size exceeds maximum allowed file size " + + std::to_string(max_external_file_size_) + "."; + } + return false; + } + is_binary_ = false; bin_data_ = nullptr; bin_size_ = 0; @@ -6724,6 +6815,30 @@ bool TinyGLTF::LoadASCIIFromFile(Model *model, std::string *err, return false; } + if (fs.GetFileSizeInBytes) { + size_t file_size{0}; + std::string file_size_err; + bool ok = fs.GetFileSizeInBytes(&file_size, &file_size_err, filename, + fs.user_data); + if (!ok) { + ss << "Failed to stat file: " << filename << ": " << file_size_err + << std::endl; + if (err) { + (*err) = ss.str(); + } + return false; + } + if (file_size > max_external_file_size_) { + ss << "File size " << file_size + << " exceeds maximum allowed file size " << max_external_file_size_ + << " : " << filename << std::endl; + if (err) { + (*err) = ss.str(); + } + return false; + } + } + std::vector data; std::string fileerr; bool fileread = fs.ReadWholeFile(&data, &fileerr, filename, fs.user_data); @@ -6742,6 +6857,14 @@ bool TinyGLTF::LoadASCIIFromFile(Model *model, std::string *err, } return false; } + if (sz > max_external_file_size_) { + if (err) { + (*err) = "File size " + std::to_string(sz) + + " exceeds maximum allowed file size " + + std::to_string(max_external_file_size_) + " : " + filename; + } + return false; + } std::string basedir = GetBaseDir(filename); @@ -6758,6 +6881,14 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err, unsigned int size, const std::string &base_dir, unsigned int check_sections) { + if (size > max_external_file_size_) { + if (err) { + (*err) = "Input size exceeds maximum allowed file size " + + std::to_string(max_external_file_size_) + "."; + } + return false; + } + if (size < 20) { if (err) { (*err) = "Too short data size for glTF Binary."; @@ -6816,6 +6947,29 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err, return false; } + if (length != size) { + if (strictness_ == ParseStrictness::Permissive) { + if (warn) { + (*warn) += + "GLB length field does not match actual data size. Trailing data " + "will be ignored.\n"; + } + } else { + if (err) { + (*err) = "Invalid glTF binary. Length field does not match data size."; + } + return false; + } + } + + if (chunk0_length > max_external_file_size_) { + if (err) { + (*err) = "JSON chunk size exceeds maximum allowed file size " + + std::to_string(max_external_file_size_) + "."; + } + return false; + } + // Padding check // The start and the end of each chunk must be aligned to a 4-byte boundary. // No padding check for chunk0 start since its 4byte-boundary is ensured. @@ -6950,6 +7104,30 @@ bool TinyGLTF::LoadBinaryFromFile(Model *model, std::string *err, return false; } + if (fs.GetFileSizeInBytes) { + size_t file_size{0}; + std::string file_size_err; + bool ok = fs.GetFileSizeInBytes(&file_size, &file_size_err, filename, + fs.user_data); + if (!ok) { + ss << "Failed to stat file: " << filename << ": " << file_size_err + << std::endl; + if (err) { + (*err) = ss.str(); + } + return false; + } + if (file_size > max_external_file_size_) { + ss << "File size " << file_size + << " exceeds maximum allowed file size " << max_external_file_size_ + << " : " << filename << std::endl; + if (err) { + (*err) = ss.str(); + } + return false; + } + } + std::vector data; std::string fileerr; bool fileread = fs.ReadWholeFile(&data, &fileerr, filename, fs.user_data); @@ -6961,6 +7139,22 @@ bool TinyGLTF::LoadBinaryFromFile(Model *model, std::string *err, return false; } + size_t sz = data.size(); + if (sz == 0) { + if (err) { + (*err) = "Empty file."; + } + return false; + } + if (sz > max_external_file_size_) { + if (err) { + (*err) = "File size " + std::to_string(sz) + + " exceeds maximum allowed file size " + + std::to_string(max_external_file_size_) + " : " + filename; + } + return false; + } + std::string basedir = GetBaseDir(filename); bool ret = LoadBinaryFromMemory(model, err, warn, &data.at(0),