Merge pull request #467 from rhiskey/release

Small security and overflow patch

.github/workflows/mingw-w64-msys2.yml

@@ -0,0 +1,45 @@
+name: MSYS2 MinGW-w64 Windows 64bit Build
+
+on:
+  push:
+    branches:
+      - release
+      - devel
+    paths:
+    - 'tiny_gltf.*'
+    - 'CMakeLists.txt'
+    - '.github/workflows/mingw-w64-msys2.yml'
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  mingw-w64-msys2-build:
+    name: MSYS2 MinGW-w64 Windows Build
+    runs-on: windows-latest
+    defaults:
+      run:
+        shell: msys2 {0}
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Install core & build dependencies
+        uses: msys2/setup-msys2@v2
+        with:
+          msystem: UCRT64
+          install: base-devel
+          pacboy: >-
+            cc:p cmake:p ninja:p
+          update: true
+          release: false
+
+      - name: Configure
+        run: |
+          cmake \
+            -G"Ninja" \
+            -S . \
+            -B build
+
+      - name: Build
+        run: |
+          cmake --build build
+

README.md

@@ -109,6 +109,7 @@ WASI build example is located in [wasm](wasm) .
 * [SanityEngine](https://github.com/DethRaid/SanityEngine) - A C++/D3D12 renderer focused on the personal and professional development of its developer
 * [Open3D](http://www.open3d.org/) - A Modern Library for 3D Data Processing
 * [Supernova Engine](https://github.com/supernovaengine/supernova) - Game engine for 2D and 3D projects with Lua or C++ in data oriented design.
+* [Wicked Engine<img src="https://github.com/turanszkij/WickedEngine/blob/master/Content/logo_small.png" width="28px" align="center"/>](https://github.com/turanszkij/WickedEngine) - 3D engine with modern graphics 
 * Your projects here! (Please send PR)
 
 ## TODOs

stb_image_write.h

@@ -773,7 +773,7 @@ static int stbi_write_hdr_core(stbi__write_context *s, int x, int y, int comp, f
 #ifdef __STDC_LIB_EXT1__
       len = sprintf_s(buffer, sizeof(buffer), "EXPOSURE=          1.0000000000000\n\n-Y %d +X %d\n", y, x);
 #else
-      len = sprintf(buffer, "EXPOSURE=          1.0000000000000\n\n-Y %d +X %d\n", y, x);
+      len = snprintf(buffer, sizeof(buffer), "EXPOSURE=          1.0000000000000\n\n-Y %d +X %d\n", y, x);
 #endif
       s->func(s->context, buffer, len);
 

tests/tester.cc

@@ -494,25 +494,23 @@ TEST_CASE("image-uri-spaces", "[issue-236]") {
 }
 
 TEST_CASE("serialize-empty-material", "[issue-294]") {
-
   tinygltf::Model m;
-
-  tinygltf::Material mat;
-  mat.pbrMetallicRoughness.baseColorFactor = {1.0f, 1.0f, 1.0f, 1.0f}; // default baseColorFactor
-  m.materials.push_back(mat);
-
+  // Add default constructed material to model
+  m.materials.push_back({});
+  // Serialize model to output stream
   std::stringstream os;
-
   tinygltf::TinyGLTF ctx;
   bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false);
   REQUIRE(true == ret);
-
-  // use nlohmann json
+  // Parse serialized model
   nlohmann::json j = nlohmann::json::parse(os.str());
-
+  // Serialized materials shall hold an empty object that
+  // represents the default constructed material
+  REQUIRE(j.find("materials") != j.end());
+  REQUIRE(j["materials"].is_array());
   REQUIRE(1 == j["materials"].size());
-  REQUIRE(j["materials"][0].is_object());
-
+  CHECK(j["materials"][0].is_object());
+  CHECK(j["materials"][0].empty());
 }
 
 TEST_CASE("empty-skeleton-id", "[issue-321]") {
@@ -757,3 +755,150 @@ TEST_CASE("load-issue-416-model", "[issue-416]") {
   // external file load fails, but reading glTF itself is ok.
   REQUIRE(true == ret);
 }
+
+TEST_CASE("serialize-empty-node", "[issue-457]") {
+  tinygltf::Model m;
+  // Add default constructed node to model
+  m.nodes.push_back({});
+  // Add scene to model
+  m.scenes.push_back({});
+  // The scene's only node is the empty node
+  m.scenes.front().nodes.push_back(0);
+
+  // Serialize model to output stream
+  std::stringstream os;
+  tinygltf::TinyGLTF ctx;
+  bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false);
+  REQUIRE(true == ret);
+
+  // Parse serialized model
+  nlohmann::json j = nlohmann::json::parse(os.str());
+
+  // Serialized nodes shall hold an empty object that
+  // represents the default constructed node
+  REQUIRE(j.find("nodes") != j.end());
+  REQUIRE(j["nodes"].is_array());
+  REQUIRE(1 == j["nodes"].size());
+  CHECK(j["nodes"][0].is_object());
+  CHECK(j["nodes"][0].empty());
+
+  // We also want to make sure that the serialized scene
+  // is referencing the empty node.
+
+  // There shall be a single serialized scene
+  auto scenes = j.find("scenes");
+  REQUIRE(scenes != j.end());
+  REQUIRE(scenes->is_array());
+  REQUIRE(1 == scenes->size());
+  auto scene = scenes->at(0);
+  REQUIRE(scene.is_object());
+  // The scene's nodes array shall hold a reference
+  // to the single node
+  auto nodes = scene.find("nodes");
+  REQUIRE(nodes != scene.end());
+  REQUIRE(nodes->is_array());
+  REQUIRE(1 == nodes->size());
+  auto node = nodes->at(0);
+  CHECK(node.is_number_integer());
+  int idx = -1;
+  node.get_to(idx);
+  CHECK(0 == idx);
+}
+
+TEST_CASE("serialize-light-index", "[issue-458]") {
+
+  // Create the light
+  tinygltf::Light light;
+  light.type = "point";
+  light.intensity = 0.75;
+  light.color = std::vector<double>{1.0, 0.8, 0.95};
+
+  // Stream to serialize to
+  std::stringstream os;
+
+  {
+    tinygltf::Model m;
+    tinygltf::Scene scene;
+    // Add the light to the model
+    m.lights.push_back(light);
+    // Create a node that uses the light
+    tinygltf::Node node;
+    node.light = 0;
+    // Add the node to the model
+    m.nodes.push_back(node);
+    // Add the node to the scene
+    scene.nodes.push_back(0);
+    // Add the scene to the model
+    m.scenes.push_back(scene);
+    // Serialize model to output stream
+    tinygltf::TinyGLTF ctx;
+    bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false);
+    REQUIRE(true == ret);
+  }
+
+  {
+    tinygltf::Model m;
+    tinygltf::TinyGLTF ctx;
+    // Parse the serialized model
+    bool ok = ctx.LoadASCIIFromString(&m, nullptr, nullptr, os.str().c_str(), os.str().size(), "");
+    REQUIRE(true == ok);
+    // Check if the light was correctly serialized
+    REQUIRE(1 == m.lights.size());
+    CHECK(m.lights[0] == light);
+    // Check that the node properly references the light
+    REQUIRE(1 == m.nodes.size());
+    CHECK(m.nodes[0].light == 0);
+  }
+}
+
+TEST_CASE("default-material", "[issue-459]") {
+  const std::vector<double> default_emissive_factor{ 0.0, 0.0, 0.0 };
+  const std::vector<double> default_base_color_factor{ 1.0, 1.0, 1.0, 1.0 };
+  const std::string default_alpha_mode = "OPAQUE";
+  const double default_alpha_cutoff = 0.5;
+  const bool default_double_sided = false;
+  const double default_metallic_factor = 1.0;
+  const double default_roughness_factor = 1.0;
+  // Check that default constructed material
+  // holds actual default GLTF material properties
+  tinygltf::Material mat;
+  CHECK(mat.alphaMode == default_alpha_mode);
+  CHECK(mat.alphaCutoff == default_alpha_cutoff);
+  CHECK(mat.doubleSided == default_double_sided);
+  CHECK(mat.emissiveFactor == default_emissive_factor);
+  CHECK(mat.pbrMetallicRoughness.baseColorFactor == default_base_color_factor);
+  CHECK(mat.pbrMetallicRoughness.metallicFactor == default_metallic_factor);
+  CHECK(mat.pbrMetallicRoughness.roughnessFactor == default_roughness_factor);
+  // None of the textures should be set
+  CHECK(mat.normalTexture.index == -1);
+  CHECK(mat.occlusionTexture.index == -1);
+  CHECK(mat.emissiveTexture.index == -1);
+}
+
+TEST_CASE("serialize-empty-scene", "[issue-464]") {
+  // Stream to serialize to
+  std::stringstream os;
+
+  {
+    tinygltf::Model m;
+    // Add empty scene to the model
+    m.scenes.push_back({});
+    // Serialize model to output stream
+    tinygltf::TinyGLTF ctx;
+    bool ret = ctx.WriteGltfSceneToStream(&m, os, false, false);
+    REQUIRE(true == ret);
+  }
+
+  {
+    tinygltf::Model m;
+    tinygltf::TinyGLTF ctx;
+    // Parse the serialized model
+    bool ok = ctx.LoadASCIIFromString(&m, nullptr, nullptr, os.str().c_str(), os.str().size(), "");
+    REQUIRE(true == ok);
+    // Make sure the empty scene is there
+    REQUIRE(1 == m.scenes.size());
+    tinygltf::Scene scene{};
+    // Check that the scene is empty
+    CHECK(m.scenes[0] == scene);
+  }
+}

tiny_gltf.h

@@ -195,6 +195,11 @@ typedef enum {
   OBJECT_TYPE
 } Type;
 
+typedef enum {
+  Permissive,
+  Strict
+} ParseStrictness;
+
 static inline int32_t GetComponentSizeInBytes(uint32_t componentType) {
   if (componentType == TINYGLTF_COMPONENT_TYPE_BYTE) {
     return 1;
@@ -730,7 +735,7 @@ struct OcclusionTextureInfo {
 
 // pbrMetallicRoughness class defined in glTF 2.0 spec.
 struct PbrMetallicRoughness {
-  std::vector<double> baseColorFactor;  // len = 4. default [1,1,1,1]
+  std::vector<double> baseColorFactor{1.0, 1.0, 1.0, 1.0};  // len = 4. default [1,1,1,1]
   TextureInfo baseColorTexture;
   double metallicFactor{1.0};   // default 1
   double roughnessFactor{1.0};  // default 1
@@ -743,9 +748,9 @@ struct PbrMetallicRoughness {
   std::string extras_json_string;
   std::string extensions_json_string;
 
-  PbrMetallicRoughness()
-      : baseColorFactor(std::vector<double>{1.0, 1.0, 1.0, 1.0}) {}
+  PbrMetallicRoughness() = default;
   DEFAULT_METHODS(PbrMetallicRoughness)
+
   bool operator==(const PbrMetallicRoughness &) const;
 };
 
@@ -755,10 +760,10 @@ struct PbrMetallicRoughness {
 struct Material {
   std::string name;
 
-  std::vector<double> emissiveFactor;  // length 3. default [0, 0, 0]
-  std::string alphaMode;               // default "OPAQUE"
-  double alphaCutoff{0.5};             // default 0.5
-  bool doubleSided{false};             // default false;
+  std::vector<double> emissiveFactor{0.0, 0.0, 0.0};  // length 3. default [0, 0, 0]
+  std::string alphaMode{"OPAQUE"}; // default "OPAQUE"
+  double alphaCutoff{0.5};        // default 0.5
+  bool doubleSided{false};        // default false
 
   PbrMetallicRoughness pbrMetallicRoughness;
 
@@ -778,7 +783,7 @@ struct Material {
   std::string extras_json_string;
   std::string extensions_json_string;
 
-  Material() : alphaMode("OPAQUE") {}
+  Material() = default;
   DEFAULT_METHODS(Material)
 
   bool operator==(const Material &) const;
@@ -832,7 +837,7 @@ struct Accessor {
     int count;
     bool isSparse;
     struct {
-      int byteOffset;
+      size_t byteOffset;
       int bufferView;
       int componentType;  // a TINYGLTF_COMPONENT_TYPE_ value
       Value extras;
@@ -842,7 +847,7 @@ struct Accessor {
     } indices;
     struct {
       int bufferView;
-      int byteOffset;
+      size_t byteOffset;
       Value extras;
       ExtensionMap extensions;
       std::string extras_json_string;
@@ -1463,6 +1468,11 @@ class TinyGLTF {
                             bool embedImages, bool embedBuffers,
                             bool prettyPrint, bool writeBinary);
 
+  ///
+  /// Sets the parsing strictness.
+  ///
+  void SetParseStrictness(ParseStrictness strictness);
+
   ///
   /// Set callback to use for loading image data
   ///
@@ -1552,6 +1562,8 @@ class TinyGLTF {
   size_t bin_size_ = 0;
   bool is_binary_ = false;
 
+  ParseStrictness strictness_ = ParseStrictness::Strict;
+
   bool serialize_default_values_ = false;  ///< Serialize default values?
 
   bool store_original_json_for_extras_and_extensions_ = false;
@@ -2553,6 +2565,10 @@ static bool LoadExternalFile(std::vector<unsigned char> *out, std::string *err,
   return true;
 }
 
+void TinyGLTF::SetParseStrictness(ParseStrictness strictness) {
+  strictness_ = strictness;
+}
+
 void TinyGLTF::SetImageLoader(LoadImageDataFunction func, void *user_data) {
   LoadImageData = func;
   load_image_user_data_ = user_data;
@@ -4632,24 +4648,26 @@ static bool ParseSparseAccessor(
   const detail::json &indices_obj = detail::GetValue(indices_iterator);
   const detail::json &values_obj = detail::GetValue(values_iterator);
 
-  int indices_buffer_view = 0, indices_byte_offset = 0, component_type = 0;
+  int indices_buffer_view = 0, component_type = 0;
+  size_t indices_byte_offset = 0;
   if (!ParseIntegerProperty(&indices_buffer_view, err, indices_obj,
                             "bufferView", true, "SparseAccessor")) {
     return false;
   }
-  ParseIntegerProperty(&indices_byte_offset, err, indices_obj, "byteOffset",
+  ParseUnsignedProperty(&indices_byte_offset, err, indices_obj, "byteOffset",
                        false);
   if (!ParseIntegerProperty(&component_type, err, indices_obj, "componentType",
                             true, "SparseAccessor")) {
     return false;
   }
 
-  int values_buffer_view = 0, values_byte_offset = 0;
+  int values_buffer_view = 0;
+  size_t values_byte_offset = 0;
   if (!ParseIntegerProperty(&values_buffer_view, err, values_obj, "bufferView",
                             true, "SparseAccessor")) {
     return false;
   }
-  ParseIntegerProperty(&values_byte_offset, err, values_obj, "byteOffset",
+  ParseUnsignedProperty(&values_byte_offset, err, values_obj, "byteOffset",
                        false);
 
   sparse->count = count;
@@ -4857,8 +4875,9 @@ static bool GetAttributeForAllPoints(uint32_t componentType, draco::Mesh *mesh,
 }
 
 static bool ParseDracoExtension(Primitive *primitive, Model *model,
-                                std::string *err,
-                                const Value &dracoExtensionValue) {
+                                std::string *err, std::string *warn,
+                                const Value &dracoExtensionValue,
+                                ParseStrictness strictness) {
   (void)err;
   auto bufferViewValue = dracoExtensionValue.Get("bufferView");
   if (!bufferViewValue.IsInt()) return false;
@@ -4890,6 +4909,33 @@ static bool ParseDracoExtension(Primitive *primitive, Model *model,
 
   // create new bufferView for indices
   if (primitive->indices >= 0) {
+    if (strictness == ParseStrictness::Permissive) {
+      const draco::PointIndex::ValueType numPoint = mesh->num_points();
+      // handle the situation where the stored component type does not match the
+      // required type for the actual number of stored points
+      int supposedComponentType = TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE;
+      if (numPoint < static_cast<draco::PointIndex::ValueType>(
+                         std::numeric_limits<uint8_t>::max())) {
+        supposedComponentType = TINYGLTF_COMPONENT_TYPE_UNSIGNED_BYTE;
+      } else if (
+          numPoint < static_cast<draco::PointIndex::ValueType>(
+                         std::numeric_limits<uint16_t>::max())) {
+        supposedComponentType = TINYGLTF_COMPONENT_TYPE_UNSIGNED_SHORT;
+      } else {
+        supposedComponentType = TINYGLTF_COMPONENT_TYPE_UNSIGNED_INT;
+      }
+
+      if (supposedComponentType > model->accessors[primitive->indices].componentType) {
+        if (warn) {
+          (*warn) +=
+              "GLTF component type " + std::to_string(model->accessors[primitive->indices].componentType) +
+              " is not sufficient for number of stored points,"
+              " treating as " + std::to_string(supposedComponentType) + "\n";
+        }
+        model->accessors[primitive->indices].componentType = supposedComponentType;
+      }
+    }
+
     int32_t componentSize = GetComponentSizeInBytes(
         model->accessors[primitive->indices].componentType);
     Buffer decodedIndexBuffer;
@@ -4955,9 +5001,11 @@ static bool ParseDracoExtension(Primitive *primitive, Model *model,
 }
 #endif
 
-static bool ParsePrimitive(Primitive *primitive, Model *model, std::string *err,
+static bool ParsePrimitive(Primitive *primitive, Model *model,
+                           std::string *err, std::string *warn,
                            const detail::json &o,
-                           bool store_original_json_for_extras_and_extensions) {
+                           bool store_original_json_for_extras_and_extensions,
+                           ParseStrictness strictness) {
   int material = -1;
   ParseIntegerProperty(&material, err, o, "material", false);
   primitive->material = material;
@@ -5006,18 +5054,22 @@ static bool ParsePrimitive(Primitive *primitive, Model *model, std::string *err,
   auto dracoExtension =
       primitive->extensions.find("KHR_draco_mesh_compression");
   if (dracoExtension != primitive->extensions.end()) {
-    ParseDracoExtension(primitive, model, err, dracoExtension->second);
+    ParseDracoExtension(primitive, model, err, warn, dracoExtension->second, strictness);
   }
 #else
   (void)model;
+  (void)warn;
+  (void)strictness;
 #endif
 
   return true;
 }
 
-static bool ParseMesh(Mesh *mesh, Model *model, std::string *err,
+static bool ParseMesh(Mesh *mesh, Model *model,
+                      std::string *err, std::string *warn,
                       const detail::json &o,
-                      bool store_original_json_for_extras_and_extensions) {
+                      bool store_original_json_for_extras_and_extensions,
+                      ParseStrictness strictness) {
   ParseStringProperty(&mesh->name, err, o, "name", false);
 
   mesh->primitives.clear();
@@ -5030,8 +5082,9 @@ static bool ParseMesh(Mesh *mesh, Model *model, std::string *err,
              detail::ArrayBegin(detail::GetValue(primObject));
          i != primEnd; ++i) {
       Primitive primitive;
-      if (ParsePrimitive(&primitive, model, err, *i,
-                         store_original_json_for_extras_and_extensions)) {
+      if (ParsePrimitive(&primitive, model, err, warn, *i,
+                         store_original_json_for_extras_and_extensions,
+                         strictness)) {
         // Only add the primitive if the parsing succeeds.
         mesh->primitives.emplace_back(std::move(primitive));
       }
@@ -5192,15 +5245,24 @@ static bool ParsePbrMetallicRoughness(
   return true;
 }
 
-static bool ParseMaterial(Material *material, std::string *err,
+static bool ParseMaterial(Material *material, std::string *err, std::string *warn,
                           const detail::json &o,
-                          bool store_original_json_for_extras_and_extensions) {
+                          bool store_original_json_for_extras_and_extensions,
+                          ParseStrictness strictness) {
   ParseStringProperty(&material->name, err, o, "name", /* required */ false);
 
   if (ParseNumberArrayProperty(&material->emissiveFactor, err, o,
                                "emissiveFactor",
                                /* required */ false)) {
-    if (material->emissiveFactor.size() != 3) {
+    if (strictness==ParseStrictness::Permissive && material->emissiveFactor.size() == 4) {
+      if (warn) {
+        (*warn) +=
+            "Array length of `emissiveFactor` parameter in "
+            "material must be 3, but got 4\n";
+      }
+      material->emissiveFactor.resize(3);
+    }
+    else if (material->emissiveFactor.size() != 3) {
       if (err) {
         (*err) +=
             "Array length of `emissiveFactor` parameter in "
@@ -6053,8 +6115,9 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn,
         return false;
       }
       Mesh mesh;
-      if (!ParseMesh(&mesh, model, err, o,
-                     store_original_json_for_extras_and_extensions_)) {
+      if (!ParseMesh(&mesh, model, err, warn, o,
+                     store_original_json_for_extras_and_extensions_,
+                     strictness_)) {
         return false;
       }
 
@@ -6082,20 +6145,22 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn,
           return false;
         }
 
-        auto bufferView =
+        const auto bufferView =
             model->accessors[size_t(primitive.indices)].bufferView;
-        if (bufferView < 0 || size_t(bufferView) >= model->bufferViews.size()) {
+        if (bufferView < 0) {
+          // skip, bufferView could be null(-1) for certain extensions
+        } else if (size_t(bufferView) >= model->bufferViews.size()) {
           if (err) {
             (*err) += "accessor[" + std::to_string(primitive.indices) +
                       "] invalid bufferView";
           }
           return false;
+        } else {
+          model->bufferViews[size_t(bufferView)].target =
+              TINYGLTF_TARGET_ELEMENT_ARRAY_BUFFER;
+          // we could optionally check if accessors' bufferView type is Scalar, as
+          // it should be
         }
-
-        model->bufferViews[size_t(bufferView)].target =
-            TINYGLTF_TARGET_ELEMENT_ARRAY_BUFFER;
-        // we could optionally check if accessors' bufferView type is Scalar, as
-        // it should be
       }
 
       for (auto &attribute : primitive.attributes) {
@@ -6198,8 +6263,9 @@ bool TinyGLTF::LoadFromString(Model *model, std::string *err, std::string *warn,
       Material material;
       ParseStringProperty(&material.name, err, o, "name", false);
 
-      if (!ParseMaterial(&material, err, o,
-                         store_original_json_for_extras_and_extensions_)) {
+      if (!ParseMaterial(&material, err, warn, o,
+                         store_original_json_for_extras_and_extensions_,
+                         strictness_)) {
         return false;
       }
 
@@ -6603,9 +6669,12 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err,
   // Use 64bit uint to avoid integer overflow.
   uint64_t header_and_json_size = 20ull + uint64_t(chunk0_length);
 
-  if (header_and_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 (err) {
+      (*err) = "Invalid glTF binary. GLB data exceeds 4GB.";
+    }
+    return false;
   }
 
   if ((header_and_json_size > uint64_t(size)) || (chunk0_length < 1) ||
@@ -6624,6 +6693,7 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err,
     if (err) {
       (*err) = "JSON Chunk end does not aligned to a 4-byte boundary.";
     }
+    return false;
   }
 
   // std::cout << "header_and_json_size = " << header_and_json_size << "\n";
@@ -6644,8 +6714,8 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err,
       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)) +
+            "Must have 4 or more bytes, but got " +
+            std::to_string((header_and_json_size + 8ull) - uint64_t(length)) +
             ".\n";
       }
       return false;
@@ -6669,10 +6739,17 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err,
     }
 
     if ((chunk1_length % 4) != 0) {
-      if (err) {
-        (*err) = "BIN Chunk end does not aligned to a 4-byte boundary.";
+      if (strictness_==ParseStrictness::Permissive) {
+        if (warn) {
+          (*warn) += "BIN Chunk end is not aligned to a 4-byte boundary.\n";
+        }
+      }
+      else {
+        if (err) {
+          (*err) = "BIN Chunk end is not aligned to a 4-byte boundary.";
+        }
+        return false;
       }
-      return false;
     }
 
     if (uint64_t(chunk1_length) + header_and_json_size > uint64_t(length)) {
@@ -6697,10 +6774,6 @@ bool TinyGLTF::LoadBinaryFromMemory(Model *model, std::string *err,
     bin_size_ = size_t(chunk1_length);
   }
 
-  // Extract JSON string.
-  std::string jsonString(reinterpret_cast<const char *>(&bytes[20]),
-                         chunk0_length);
-
   is_binary_ = true;
 
   bool ret = LoadFromString(model, err, warn,
@@ -7098,7 +7171,7 @@ static void SerializeGltfAccessor(const Accessor &accessor, detail::json &o) {
     SerializeNumberProperty<int>("bufferView", accessor.bufferView, o);
 
   if (accessor.byteOffset != 0)
-    SerializeNumberProperty<int>("byteOffset", int(accessor.byteOffset), o);
+    SerializeNumberProperty<size_t>("byteOffset", accessor.byteOffset, o);
 
   SerializeNumberProperty<int>("componentType", accessor.componentType, o);
   SerializeNumberProperty<size_t>("count", accessor.count, o);
@@ -7169,7 +7242,7 @@ static void SerializeGltfAccessor(const Accessor &accessor, detail::json &o) {
       detail::json indices;
       SerializeNumberProperty<int>("bufferView",
                                    accessor.sparse.indices.bufferView, indices);
-      SerializeNumberProperty<int>("byteOffset",
+      SerializeNumberProperty<size_t>("byteOffset",
                                    accessor.sparse.indices.byteOffset, indices);
       SerializeNumberProperty<int>(
           "componentType", accessor.sparse.indices.componentType, indices);
@@ -7180,7 +7253,7 @@ static void SerializeGltfAccessor(const Accessor &accessor, detail::json &o) {
       detail::json values;
       SerializeNumberProperty<int>("bufferView",
                                    accessor.sparse.values.bufferView, values);
-      SerializeNumberProperty<int>("byteOffset",
+      SerializeNumberProperty<size_t>("byteOffset",
                                    accessor.sparse.values.byteOffset, values);
       SerializeExtrasAndExtensions(accessor.sparse.values, values);
       detail::JsonAddMember(sparse, "values", std::move(values));
@@ -7683,7 +7756,7 @@ static void SerializeGltfNode(const Node &node, detail::json &o) {
       detail::JsonSetObject(lights_punctual);
       detail::JsonAddMember(extensions, "KHR_lights_punctual",
                             std::move(lights_punctual));
-      detail::FindMember(o, "KHR_lights_punctual", it);
+      detail::FindMember(extensions, "KHR_lights_punctual", it);
     }
     SerializeNumberProperty("light", node.light, detail::GetValue(it));
   } else {
@@ -7967,6 +8040,16 @@ static void SerializeGltfModel(const Model *model, detail::json &o) {
     for (unsigned int i = 0; i < model->nodes.size(); ++i) {
       detail::json node;
       SerializeGltfNode(model->nodes[i], node);
+
+      if (detail::JsonIsNull(node)) {
+        // Issue 457.
+        // `node` does not have any required parameters,
+        // so the result may be null(unmodified) when all node parameters
+        // have default value.
+        //
+        // null is not allowed thus we create an empty JSON object.
+        detail::JsonSetObject(node);
+      }
       detail::JsonPushBack(nodes, std::move(node));
     }
     detail::JsonAddMember(o, "nodes", std::move(nodes));
@@ -7984,6 +8067,15 @@ static void SerializeGltfModel(const Model *model, detail::json &o) {
     for (unsigned int i = 0; i < model->scenes.size(); ++i) {
       detail::json currentScene;
       SerializeGltfScene(model->scenes[i], currentScene);
+      if (detail::JsonIsNull(currentScene)) {
+        // Issue 464.
+        // `scene` does not have any required parameters,
+        // so the result may be null(unmodified) when all scene parameters
+        // have default value.
+        //
+        // null is not allowed thus we create an empty JSON object.
+        detail::JsonSetObject(currentScene);
+      }
       detail::JsonPushBack(scenes, std::move(currentScene));
     }
     detail::JsonAddMember(o, "scenes", std::move(scenes));