v2.6.3

Fix parsing GLB file with empty Chunk1(BIN data).

.github/workflows/codeql-analysis.yml

@@ -0,0 +1,72 @@
+# For most projects, this workflow file will not need changing; you simply need
+# to commit it to your repository.
+#
+# You may wish to alter this file to override the set of languages analyzed,
+# or to provide custom queries or build logic.
+#
+# ******** NOTE ********
+# We have attempted to detect the languages in your repository. Please check
+# the `language` matrix defined below to confirm you have the correct set of
+# supported CodeQL languages.
+#
+name: "CodeQL"
+
+on:
+  push:
+    branches: [ "master" ]
+  pull_request:
+    # The branches below must be a subset of the branches above
+    branches: [ "master" ]
+  schedule:
+    - cron: '21 20 * * 5'
+
+jobs:
+  analyze:
+    name: Analyze
+    runs-on: ubuntu-latest
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    strategy:
+      fail-fast: false
+      matrix:
+        language: [ 'cpp', 'python' ]
+        # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ]
+        # Learn more about CodeQL language support at https://aka.ms/codeql-docs/language-support
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v3
+
+    # Initializes the CodeQL tools for scanning.
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v2
+      with:
+        languages: ${{ matrix.language }}
+        # If you wish to specify custom queries, you can do so here or in a config file.
+        # By default, queries listed here will override any specified in a config file.
+        # Prefix the list here with "+" to use these queries and those in the config file.
+        
+        # Details on CodeQL's query packs refer to : https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
+        # queries: security-extended,security-and-quality
+
+        
+    # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
+    # If this step fails, then you should remove it and run the build manually (see below)
+    - name: Autobuild
+      uses: github/codeql-action/autobuild@v2
+
+    # ℹ️ Command-line programs to run using the OS shell.
+    # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
+
+    #   If the Autobuild fails above, remove it and uncomment the following three lines. 
+    #   modify them (or add more) to build your code if your project, please refer to the EXAMPLE below for guidance.
+
+    # - run: |
+    #   echo "Run, Build Application using script"
+    #   ./location_of_script_within_repo/buildscript.sh
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v2

.gitignore

@@ -68,4 +68,11 @@ loader_example
 tests/tester
 tests/tester_noexcept
 tests/issue-97.gltf
+tests/issue-261.gltf
 
+# unignore
+!Makefile
+!examples/build-gltf/Makefile
+!examples/raytrace/cornellbox_suzanne.obj
+!tests/Makefile
+!tools/windows/premake5.exe

tests/tester.cc

@@ -482,3 +482,61 @@ TEST_CASE("expandpath-utf-8", "[pr-226]") {
 
 }
 #endif
+
+TEST_CASE("empty-bin-buffer", "[issue-382]") {
+  tinygltf::Model model;
+  tinygltf::TinyGLTF ctx;
+  std::string err;
+  std::string warn;
+
+  tinygltf::Model model_empty;
+  std::stringstream stream;
+  bool ret = ctx.WriteGltfSceneToStream(&model_empty, stream, false, true);
+  REQUIRE(ret == true);
+  std::string str = stream.str();
+  const unsigned char* bytes = (unsigned char*)str.data();
+  ret = ctx.LoadBinaryFromMemory(&model, &err, &warn, bytes, str.size());
+  if (!err.empty()) {
+    std::cerr << err << std::endl;
+  }
+  REQUIRE(true == ret);
+
+  err.clear();
+  warn.clear();
+
+  tinygltf::Model model_empty_buffer;
+  model_empty_buffer.buffers.push_back(tinygltf::Buffer());
+  stream = std::stringstream();
+  ret = ctx.WriteGltfSceneToStream(&model_empty_buffer, stream, false, true);
+  REQUIRE(ret == true);
+  str = stream.str();
+  bytes = (unsigned char*)str.data();
+  ret = ctx.LoadBinaryFromMemory(&model, &err, &warn, bytes, str.size());
+  if (err.empty()) {
+    std::cerr << "there should have been an error reported" << std::endl;
+  }
+  REQUIRE(false == ret);
+
+  err.clear();
+  warn.clear();
+
+  tinygltf::Model model_single_byte_buffer;
+  tinygltf::Buffer buffer;
+  buffer.data.push_back(0);
+  model_single_byte_buffer.buffers.push_back(buffer);
+  stream = std::stringstream();
+  ret = ctx.WriteGltfSceneToStream(&model_single_byte_buffer, stream, false, true);
+  REQUIRE(ret == true);
+  str = stream.str();
+  {
+    std::ofstream ofs("tmp.glb");
+    ofs.write(str.data(), str.size());
+  }
+
+  bytes = (unsigned char*)str.data();
+  ret = ctx.LoadBinaryFromMemory(&model_single_byte_buffer, &err, &warn, bytes, str.size());
+  if (!err.empty()) {
+    std::cerr << err << std::endl;
+  }
+  REQUIRE(true == ret);
+}

tiny_gltf.h

@@ -26,6 +26,9 @@
 // THE SOFTWARE.
 
 // Version:
+//  - v2.6.3 Fix GLB file with empty BIN chunk was not handled. PR#382 and PR#383.
+//  - v2.6.2 Fix out-of-bounds access of accessors. PR#379.
+//  - v2.6.1 Better GLB validation check when loading.
 //  - v2.6.0 Support serializing sparse accessor(Thanks to @fynv).
 //           Disable expanding file path for security(no use of awkward `wordexp` anymore).
 //  - v2.5.0 Add SetPreserveImageChannels() option to load image data as is.
@@ -66,6 +69,11 @@
 #include <string>
 #include <vector>
 
+//Auto-detect C++14 standard version
+#if !defined(TINYGLTF_USE_CPP14) && defined(__cplusplus) && (__cplusplus >= 201402L)
+#define TINYGLTF_USE_CPP14
+#endif
+
 #ifndef TINYGLTF_USE_CPP14
 #include <functional>
 #endif
@@ -3229,9 +3237,11 @@ static bool ParseJsonAsValue(Value *ret, const json &o) {
       break;
   }
 #endif
+  const bool isNotNull = val.Type() != NULL_TYPE;
+
   if (ret) *ret = std::move(val);
 
-  return val.Type() != NULL_TYPE;
+  return isNotNull;
 }
 
 static bool ParseExtrasProperty(Value *ret, const json &o) {
@@ -3671,7 +3681,8 @@ static bool ParseParameterProperty(Parameter *param, std::string *err,
     // Found a number array.
     return true;
   } else if (ParseNumberProperty(&param->number_value, err, o, prop, false)) {
-    return param->has_number_value = true;
+    param->has_number_value = true;
+    return true;
   } else if (ParseJSONProperty(&param->json_double_value, err, o, prop,
                                false)) {
     return true;
@@ -3864,7 +3875,7 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err,
     image->uri = uri;
 #ifdef TINYGLTF_NO_EXTERNAL_IMAGE
     return true;
-#endif
+#else
     std::string decoded_uri = dlib::urldecode(uri);
     if (!LoadExternalFile(&img, err, warn, decoded_uri, basedir,
                           /* required */ false, /* required bytes */ 0,
@@ -3886,6 +3897,7 @@ static bool ParseImage(Image *image, const int image_idx, std::string *err,
       }
       return false;
     }
+#endif
   }
 
   if (*LoadImageData == nullptr) {
@@ -4102,7 +4114,7 @@ static bool ParseBuffer(Buffer *buffer, std::string *err, const json &o,
 
       if ((bin_size == 0) || (bin_data == nullptr)) {
         if (err) {
-          (*err) += "Invalid binary data in `Buffer'.\n";
+          (*err) += "Invalid binary data in `Buffer', or GLB with empty BIN chunk.\n";
         }
         return false;
       }
@@ -5794,20 +5806,27 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn,
       }
 
       for (auto &attribute : primitive.attributes) {
-        model
-            ->bufferViews[size_t(
-                model->accessors[size_t(attribute.second)].bufferView)]
-            .target = TINYGLTF_TARGET_ARRAY_BUFFER;
+        const auto accessorsIndex = size_t(attribute.second);
+        if (accessorsIndex < model->accessors.size()) {
+          const auto bufferView = model->accessors[accessorsIndex].bufferView;
+          // bufferView could be null(-1) for sparse morph target
+          if (bufferView >= 0 && bufferView < model->bufferViews.size()) {
+            model->bufferViews[size_t(bufferView)].target =
+                TINYGLTF_TARGET_ARRAY_BUFFER;
+          }
+        }
       }
 
       for (auto &target : primitive.targets) {
         for (auto &attribute : target) {
-          auto bufferView =
-              model->accessors[size_t(attribute.second)].bufferView;
-          // bufferView could be null(-1) for sparse morph target
-          if (bufferView >= 0) {
-            model->bufferViews[size_t(bufferView)].target =
-                TINYGLTF_TARGET_ARRAY_BUFFER;
+          const auto accessorsIndex = size_t(attribute.second);
+          if (accessorsIndex < model->accessors.size()) {
+            const auto bufferView = model->accessors[accessorsIndex].bufferView;
+            // bufferView could be null(-1) for sparse morph target
+            if (bufferView >= 0 && bufferView < model->bufferViews.size()) {
+              model->bufferViews[size_t(bufferView)].target =
+                  TINYGLTF_TARGET_ARRAY_BUFFER;
+            }
           }
         }
       }
@@ -6268,15 +6287,15 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err,
   //
   // https://github.com/syoyo/tinygltf/issues/372
   // Use 64bit uint to avoid integer overflow.
-  uint64_t json_size = 20ull + uint64_t(chunk0_length);
+  uint64_t header_and_json_size = 20ull + uint64_t(chunk0_length);
 
-  if (json_size > std::numeric_limits<uint32_t>::max()) {
+  if (header_and_json_size > std::numeric_limits<uint32_t>::max()) {
     // Do not allow 4GB or more GLB data.
    (*err) = "Invalid glTF binary. GLB data exceeds 4GB.";
   }
 
-  if ((json_size > uint64_t(size)) || (chunk0_length < 1) || (length > size) ||
-      (json_size > uint64_t(length)) ||
+  if ((header_and_json_size > uint64_t(size)) || (chunk0_length < 1) || (length > size) ||
+      (header_and_json_size > uint64_t(length)) ||
       (chunk0_format != 0x4E4F534A)) {  // 0x4E4F534A = JSON format.
     if (err) {
       (*err) = "Invalid glTF binary.";
@@ -6287,61 +6306,77 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err,
   // 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.
-  if ((json_size % 4) != 0) {
+  if ((header_and_json_size % 4) != 0) {
     if (err) {
       (*err) = "JSON Chunk end does not aligned to a 4-byte boundary.";
     }
   }
 
-  // Read Chunk1 info(BIN data)
-  if ((json_size + 8ull) > uint64_t(length)) {
-    if (err) {
-      (*err) = "Insufficient storage space for Chunk1(BIN data).";
+  //std::cout << "header_and_json_size = " << header_and_json_size << "\n";
+  //std::cout << "length = " << length << "\n";
+
+  // Chunk1(BIN) data
+  // The spec says: When the binary buffer is empty or when it is stored by other means, this chunk SHOULD be omitted.
+  // So when header + JSON data == binary size, Chunk1 is omitted.
+  if (header_and_json_size == uint64_t(length)) {
+
+    bin_data_ = nullptr;
+    bin_size_ = 0;
+  } else {
+    // Read Chunk1 info(BIN data)
+    // At least Chunk1 should have 12 bytes(8 bytes(header) + 4 bytes(bin payload could be 1~3 bytes, but need to be aliged to 4 bytes)
+    if ((header_and_json_size + 12ull) > uint64_t(length)) {
+      if (err) {
+        (*err) = "Insufficient storage space for Chunk1(BIN data). At least Chunk1 Must have 4 bytes or more bytes, but got " + std::to_string((header_and_json_size + 12ull) - uint64_t(length)) + ".\n";
+      }
+      return false;
     }
-    return false;
-  }
 
-  unsigned int chunk1_length;  // 4 bytes
-  unsigned int chunk1_format;  // 4 bytes;
-  memcpy(&chunk1_length, bytes + json_size, 4); // JSON data length
-  swap4(&chunk1_length);
-  memcpy(&chunk1_format, bytes + json_size + 4, 4);
-  swap4(&chunk1_format);
+    unsigned int chunk1_length;  // 4 bytes
+    unsigned int chunk1_format;  // 4 bytes;
+    memcpy(&chunk1_length, bytes + header_and_json_size, 4); // JSON data length
+    swap4(&chunk1_length);
+    memcpy(&chunk1_format, bytes + header_and_json_size + 4, 4);
+    swap4(&chunk1_format);
 
-  if (chunk1_length < 4) {
-    // TODO: Do we allow 0byte BIN data?
-    if (err) {
-      (*err) = "Insufficient Chunk1(BIN) data size.";
+    //std::cout << "chunk1_length = " << chunk1_length << "\n";
+
+    if (chunk1_length < 4) {
+      if (err) {
+        (*err) = "Insufficient Chunk1(BIN) data size.";
+      }
+      return false;
     }
-    return false;
-  }
 
-  if ((chunk1_length % 4) != 0) {
-    if (err) {
-      (*err) = "BIN Chunk end does not aligned to a 4-byte boundary.";
+    if ((chunk1_length % 4) != 0) {
+      if (err) {
+        (*err) = "BIN Chunk end does not aligned to a 4-byte boundary.";
+      }
+      return false;
     }
-    return false;
-  }
 
-  if (uint64_t(chunk1_length) + json_size > uint64_t(length)) {
-    if (err) {
-      (*err) = "BIN Chunk data length exceeds the GLB size.";
+    if (uint64_t(chunk1_length) + header_and_json_size > uint64_t(length)) {
+      if (err) {
+        (*err) = "BIN Chunk data length exceeds the GLB size.";
+      }
+      return false;
     }
-    return false;
-  }
 
-  if (chunk1_format != 0x004e4942) {
-    if (err) {
-      (*err) = "Invlid type for chunk1 data.";
+    if (chunk1_format != 0x004e4942) {
+      if (err) {
+        (*err) = "Invlid type for chunk1 data.";
+      }
+      return false;
     }
-    return false;
+
+    //std::cout << "chunk1_length = " << chunk1_length << "\n";
+
+    bin_data_ = bytes + header_and_json_size +
+                8;  // 4 bytes (bin_buffer_length) + 4 bytes(bin_buffer_format)
+
+    bin_size_ = size_t(chunk1_length);
   }
 
-  bin_data_ = bytes + json_size +
-              8;  // 4 bytes (bin_buffer_length) + 4 bytes(bin_buffer_format)
-
-  bin_size_ = size_t(chunk1_length);
-
   // Extract JSON string.
   std::string jsonString(reinterpret_cast<const char *>(&bytes[20]),
                          chunk0_length);