diff --git a/models/BoundsChecking/invalid-buffer-index.gltf b/models/BoundsChecking/invalid-buffer-index.gltf new file mode 100644 index 0000000..88386ef --- /dev/null +++ b/models/BoundsChecking/invalid-buffer-index.gltf @@ -0,0 +1,50 @@ +{ + "scenes": [], + "nodes": [], + "meshes": [ + { + "primitives": [ + { + "attributes": {}, + "indices": 0 + } + ] + } + ], + "buffers": [ + { + "uri": "simpleTriangle.bin", + "byteLength": 44 + } + ], + "bufferViews": [ + { + "buffer": 0, + "byteOffset": 0, + "byteLength": 6, + "target": 34963 + }, + { + "buffer": 1, + "byteOffset": 0, + "byteLength": 6, + "target": 34963 + } + ], + "images": [ + { + "bufferView": 1, + "mimeType": "image/png" + } + ], + "accessors": [ + { + "bufferView": 0, + "componentType": 5123, + "count": 3, + "type": "SCALAR", + "max": [2], + "min": [0] + } + ] +} diff --git a/models/BoundsChecking/invalid-buffer-view-index.gltf b/models/BoundsChecking/invalid-buffer-view-index.gltf new file mode 100644 index 0000000..687fe85 --- /dev/null +++ b/models/BoundsChecking/invalid-buffer-view-index.gltf @@ -0,0 +1,33 @@ +{ + "scenes": [], + "nodes": [], + "buffers": [], + "meshes": [ + { + "primitives": [ + { + "attributes": {}, + "indices": 0 + } + ] + } + ], + "bufferViews": [ + { + "buffer": 0, + "byteOffset": 0, + "byteLength": 6, + "target": 34963 + } + ], + "accessors": [ + { + "bufferView": 1, + "componentType": 5123, + "count": 3, + "type": "SCALAR", + "max": [2], + "min": [0] + } + ] +} diff --git a/models/BoundsChecking/invalid-primitive-indices.gltf b/models/BoundsChecking/invalid-primitive-indices.gltf new file mode 100644 index 0000000..6dab589 --- /dev/null +++ b/models/BoundsChecking/invalid-primitive-indices.gltf @@ -0,0 +1,33 @@ +{ + "scenes": [], + "nodes": [], + "buffers": [], + "meshes": [ + { + "primitives": [ + { + "attributes": {}, + "indices": 1 + } + ] + } + ], + "bufferViews": [ + { + "buffer": 0, + "byteOffset": 0, + "byteLength": 6, + "target": 34963 + } + ], + "accessors": [ + { + "bufferView": 1, + "componentType": 5123, + "count": 3, + "type": "SCALAR", + "max": [2], + "min": [0] + } + ] +} diff --git a/models/BoundsChecking/simpleTriangle.bin b/models/BoundsChecking/simpleTriangle.bin new file mode 100644 index 0000000..d642500 Binary files /dev/null and b/models/BoundsChecking/simpleTriangle.bin differ diff --git a/tests/tester.cc b/tests/tester.cc index bb29e18..a07c549 100644 --- a/tests/tester.cc +++ b/tests/tester.cc @@ -82,7 +82,69 @@ TEST_CASE("extension-with-empty-object", "[issue-97]") { REQUIRE(m.materials[0].extensions.size() == 1); REQUIRE(m.materials[0].extensions.count("VENDOR_material_some_ext") == 1); } - + } +TEST_CASE("invalid-primitive-indices", "[bounds-checking]") { + tinygltf::Model model; + tinygltf::TinyGLTF ctx; + std::string err; + std::string warn; + // Loading is expected to fail, but not crash. + bool ret = ctx.LoadASCIIFromFile( + &model, &err, &warn, + "../models/BoundsChecking/invalid-primitive-indices.gltf"); + REQUIRE_THAT(err, + Catch::Contains("primitive indices accessor out of bounds")); + REQUIRE_FALSE(ret); +} + +TEST_CASE("invalid-buffer-view-index", "[bounds-checking]") { + tinygltf::Model model; + tinygltf::TinyGLTF ctx; + std::string err; + std::string warn; + + // Loading is expected to fail, but not crash. + bool ret = ctx.LoadASCIIFromFile( + &model, &err, &warn, + "../models/BoundsChecking/invalid-buffer-view-index.gltf"); + REQUIRE_THAT(err, Catch::Contains("accessor[0] invalid bufferView")); + REQUIRE_FALSE(ret); +} + +TEST_CASE("invalid-buffer-index", "[bounds-checking]") { + tinygltf::Model model; + tinygltf::TinyGLTF ctx; + std::string err; + std::string warn; + + // Loading is expected to fail, but not crash. + bool ret = ctx.LoadASCIIFromFile( + &model, &err, &warn, + "../models/BoundsChecking/invalid-buffer-index.gltf"); + REQUIRE_THAT( + err, Catch::Contains("image[0] buffer \"1\" not found in the scene.")); + REQUIRE_FALSE(ret); +} + +TEST_CASE("glb-invalid-length", "[bounds-checking]") { + tinygltf::Model model; + tinygltf::TinyGLTF ctx; + std::string err; + std::string warn; + + // This glb has a much longer length than the provided data and should fail + // initial range checks. + const unsigned char glb_invalid_length[] = "glTF" + "\x20\x00\x00\x00" "\x6c\x66\x00\x00" // + // | version | length | + "\x02\x00\x00\x00" "\x4a\x53\x4f\x4e{}"; // + // | model length | model format | + + bool ret = ctx.LoadBinaryFromMemory(&model, &err, &warn, glb_invalid_length, + sizeof(glb_invalid_length)); + REQUIRE_THAT(err, Catch::Contains("Invalid glTF binary.")); + REQUIRE_FALSE(ret); +} diff --git a/tiny_gltf.h b/tiny_gltf.h index 3b507a6..7f2e239 100644 --- a/tiny_gltf.h +++ b/tiny_gltf.h @@ -3926,8 +3926,24 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn, if (primitive.indices > -1) // has indices from parsing step, must be Element Array Buffer { - model->bufferViews[model->accessors[primitive.indices].bufferView] - .target = TINYGLTF_TARGET_ELEMENT_ARRAY_BUFFER; + if (size_t(primitive.indices) >= model->accessors.size()) { + if (err) { + (*err) += "primitive indices accessor out of bounds"; + } + return false; + } + + auto bufferView = model->accessors[primitive.indices].bufferView; + if (bufferView < 0 || size_t(bufferView) >= model->bufferViews.size()) { + if (err) { + (*err) += "accessor[" + std::to_string(primitive.indices) + + "] invalid bufferView"; + } + return false; + } + + model->bufferViews[bufferView].target = + TINYGLTF_TARGET_ELEMENT_ARRAY_BUFFER; // we could optionally check if acessors' bufferView type is Scalar, as // it should be } @@ -4080,6 +4096,15 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn, const BufferView &bufferView = model->bufferViews[size_t(image.bufferView)]; + if (size_t(bufferView.buffer) >= model->buffers.size()) { + if (err) { + std::stringstream ss; + ss << "image[" << idx << "] buffer \"" << bufferView.buffer + << "\" not found in the scene." << std::endl; + (*err) += ss.str(); + } + return false; + } const Buffer &buffer = model->buffers[size_t(bufferView.buffer)]; if (*LoadImageData == nullptr) { @@ -4371,7 +4396,7 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err, // In case the Bin buffer is not present, the size is exactly 20 + size of // JSON contents, // so use "greater than" operator. - if ((20 + model_length > size) || (model_length < 1) || + if ((20 + model_length > size) || (model_length < 1) || (length > size) || (model_format != 0x4E4F534A)) { // 0x4E4F534A = JSON format. if (err) { (*err) = "Invalid glTF binary.";