Add checks for boundary conditions for malformed glTF files

When loading untrusted glTF files, ideally an error should be returned
if the file is malformed instead of an exception/crash.  Add additional
validation for crashes found when running tinygltf under a fuzzer, and
add test cases to confirm:

1. Validate that the primitive indices value is within the
   model->accessors bounds before dereferencing.
2. Validate that the accessors bufferView index if valid.
3. Validate that the buffer's index is valid when parsing images.
4. For glb files, validate that the overall length is within the
   provided input buffer.
This commit is contained in:
Jeff McGlynn 2019-04-25 16:33:56 -07:00
parent 9446f65667
commit 8915252407
6 changed files with 207 additions and 4 deletions

View File

@ -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]
}
]
}

View File

@ -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]
}
]
}

View File

@ -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]
}
]
}

Binary file not shown.

After

Width:  |  Height:  |  Size: 44 B

View File

@ -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);
}

View File

@ -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.";