Fixed missing GLB checks for edge cases.

[All tests passing now]
vpenades · Dec 1, 2023 · ea01fa4 · ea01fa4
1 parent 5d33ff2
commit ea01fa4
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 7 deletions.
diff --git a/src/SharpGLTF.Core/Schema2/Serialization.Binary.cs b/src/SharpGLTF.Core/Schema2/Serialization.Binary.cs
@@ -92,21 +92,32 @@ public static IReadOnlyDictionary<UInt32, Byte[]> ReadBinaryFile(Stream stream)
 
             using (var binaryReader = new BinaryReader(stream, Encoding.ASCII))
             {
-                _ReadBinaryHeader(binaryReader);
+                var bodyLen = _ReadBinaryHeader(binaryReader);
+
+                // body length can actually be smaller than the stream length,
+                // in which case, the data afterwards is considered "extra data".
+                void _checkCanRead(long count)
+                {
+                    bool canRead = (bodyLen - stream.Position - count) >= 0;
+                    if (!canRead) throw new Validation.SchemaException(null, "unexpected End of GLB block");
+                }                
 
                 var chunks = new Dictionary<uint, Byte[]>();
-
+
+                _checkCanRead(4);
                 while (binaryReader._TryReadUInt32(out var chunkLength)) // keep reading until EndOfFile
                 {
                     if ((chunkLength & 3) != 0)
                     {
                         throw new Validation.SchemaException(null, $"The chunk must be padded to 4 bytes: {chunkLength}");
                     }
 
+                    _checkCanRead(4);
                     uint chunkId = binaryReader.ReadUInt32();
 
                     if (chunks.ContainsKey(chunkId)) throw new Validation.SchemaException(null, $"Duplicated chunk found {chunkId}");
 
+                    _checkCanRead(chunkLength);
                     var data = binaryReader.ReadBytes((int)chunkLength);
 
                     chunks[chunkId] = data;
@@ -119,7 +130,7 @@ public static IReadOnlyDictionary<UInt32, Byte[]> ReadBinaryFile(Stream stream)
             }
         }
 
-        private static void _ReadBinaryHeader(BinaryReader binaryReader)
+        private static long _ReadBinaryHeader(BinaryReader binaryReader)
         {
             Guard.NotNull(binaryReader, nameof(binaryReader));
 
@@ -129,7 +140,7 @@ private static void _ReadBinaryHeader(BinaryReader binaryReader)
             uint version = binaryReader.ReadUInt32();
             if (version != GLTFVERSION2) throw new Validation.SchemaException(null, $"Unknown version number: {version}");
 
-            uint length = binaryReader.ReadUInt32();
+            uint bodyLength = binaryReader.ReadUInt32(); // length if the actual glb body
 
             uint? fileLength = null;
 
@@ -143,10 +154,16 @@ private static void _ReadBinaryHeader(BinaryReader binaryReader)
                 // https://github.com/vpenades/SharpGLTF/issues/178
             }
 
-            if (fileLength.HasValue && length != fileLength.Value)
+            // actual block can be larger than the expected block size, in which case
+            // there might be extra data after the binary file.
+            // gltfvalidator only considers this as a warning.
+
+            if (fileLength.HasValue && bodyLength > fileLength.Value)
             {
-                throw new Validation.SchemaException(null, $"The specified length of the file ({length}) is not equal to the actual length of the file ({fileLength}).");
+                throw new Validation.SchemaException(null, $"The specified length of the file ({bodyLength}) is not equal to the actual length of the file ({fileLength}).");
             }
+
+            return bodyLength;
         }
 
         #endregion

diff --git a/src/SharpGLTF.Core/Schema2/Serialization.ReadContext.cs b/src/SharpGLTF.Core/Schema2/Serialization.ReadContext.cs
@@ -251,7 +251,7 @@ private static MODEL _FilterErrors((MODEL Model, Validation.ValidationResult Val
             catch (System.IO.EndOfStreamException ex)
             {
                 var vr = new Validation.ValidationResult(null, this.Validation);
-                vr.SetSchemaError(ex);
+                vr.SetSchemaError(ex);                
                 return (null, vr);
             }
             catch (Validation.SchemaException ex)

diff --git a/tests/SharpGLTF.Core.Tests/Validation/InvalidFilesTests.cs b/tests/SharpGLTF.Core.Tests/Validation/InvalidFilesTests.cs
@@ -43,6 +43,11 @@ public void CheckInvalidBinaryFiles()
             {
                 var json = System.IO.File.ReadAllText(f + ".report.json");
                 var report = GltfValidator.ValidationReport.Parse(json);
+
+                if (report.Issues.Messages.Any(item => item.Code.Contains("GLB_CHUNK_TOO_BIG")) && report.Issues.NumErrors > 0)
+                {
+                    // System.Diagnostics.Debugger.Break();
+                }
 
                 TestContext.Progress.WriteLine($"{f}...");
                 TestContext.WriteLine($"{f}...");