From df6e4232ef7f25d85e111a9f4a152487f8eaad89 Mon Sep 17 00:00:00 2001 From: Nexory Date: Sun, 30 Aug 2026 03:54:33 +0200 Subject: [PATCH] [C#] Verify the file identifier instead of skipping the check CheckBufferFromStart() gated the identifier comparison on identifier.Length == 0, so the comparison could only run when the caller passed an empty identifier. VerifyBuffer() rejects an empty identifier earlier with ArgumentException, so the branch was unreachable: a buffer carrying a wrong identifier, or none at all, verified as valid. The contract documented in the same file states the opposite: "When empty identifier is provided the identifier validation is skipped." The C++ verifier (verifier.h) and Swift (Root.swift) both check. flatc emits VerifyBuffer("", ...) for schemas that declare no file_identifier, which today throws instead of skipping. Flip the condition to identifier.Length != 0 and cover matching, mismatched, missing and empty identifiers with tests. TestVerifyingUnions finished its buffer with fbb.Finish(root), omitting the "MONS" identifier, while asserting that Monster.VerifyMonster() returns true. That assertion held only because the check never ran, so it now uses Monster.FinishMonsterBuffer(). --- net/FlatBuffers/FlatBufferVerify.cs | 2 +- .../FlatBufferVerifyIdentifierTests.cs | 81 +++++++++++++++++++ .../FlatBuffersExampleTests.cs | 2 +- 3 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 tests/FlatBuffers.Test/FlatBufferVerifyIdentifierTests.cs diff --git a/net/FlatBuffers/FlatBufferVerify.cs b/net/FlatBuffers/FlatBufferVerify.cs index e32256725d..ef6261d343 100644 --- a/net/FlatBuffers/FlatBufferVerify.cs +++ b/net/FlatBuffers/FlatBufferVerify.cs @@ -434,7 +434,7 @@ private bool CheckIndirectOffset(uint pos) private bool CheckBufferFromStart(string identifier, uint startPos, VerifyTableAction verifyAction) { if ((identifier != null) && - (identifier.Length == 0) && + (identifier.Length != 0) && ((verifier_buffer.Length < (SIZE_U_OFFSET + FILE_IDENTIFIER_LENGTH)) || (!BufferHasIdentifier(verifier_buffer, startPos, identifier)))) { return false; diff --git a/tests/FlatBuffers.Test/FlatBufferVerifyIdentifierTests.cs b/tests/FlatBuffers.Test/FlatBufferVerifyIdentifierTests.cs new file mode 100644 index 0000000000..a873f38436 --- /dev/null +++ b/tests/FlatBuffers.Test/FlatBufferVerifyIdentifierTests.cs @@ -0,0 +1,81 @@ +/* + * Copyright 2026 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; + +namespace Google.FlatBuffers.Test +{ + [FlatBuffersTestClass] + public class FlatBufferVerifyIdentifierTests + { + // The smallest real table verify action: exactly what generated code + // emits for a table without fields. + private static bool VerifyEmptyTable(Verifier verifier, uint tablePos) + { + return verifier.VerifyTableStart(tablePos) && verifier.VerifyTableEnd(tablePos); + } + + private static byte[] BuildBuffer(string identifier) + { + var fbb = new FlatBufferBuilder(64); + fbb.StartTable(0); + int root = fbb.EndTable(); + if (identifier == null) + { + fbb.Finish(root); + } + else + { + fbb.Finish(root, identifier); + } + return fbb.SizedByteArray(); + } + + private static bool Verify(byte[] buffer, string identifier) + { + var verifier = new Verifier(new ByteBuffer(buffer)); + return verifier.VerifyBuffer(identifier, false, VerifyEmptyTable); + } + + [FlatBuffersTestMethod] + public void VerifyBufferAcceptsMatchingIdentifier() + { + Assert.IsTrue(Verify(BuildBuffer("NULL"), "NULL")); + } + + [FlatBuffersTestMethod] + public void VerifyBufferRejectsMismatchedIdentifier() + { + Assert.IsFalse(Verify(BuildBuffer("NULL"), "ZZZZ")); + } + + [FlatBuffersTestMethod] + public void VerifyBufferRejectsMissingIdentifier() + { + Assert.IsFalse(Verify(BuildBuffer(null), "NULL")); + } + + [FlatBuffersTestMethod] + public void VerifyBufferSkipsCheckForEmptyIdentifier() + { + // Documented contract in FlatBufferVerify.cs: "When empty identifier + // is provided the identifier validation is skipped." flatc emits + // VerifyBuffer("", ...) for schemas that declare no file_identifier. + Assert.IsTrue(Verify(BuildBuffer(null), "")); + Assert.IsTrue(Verify(BuildBuffer("NULL"), "")); + } + } +} diff --git a/tests/FlatBuffers.Test/FlatBuffersExampleTests.cs b/tests/FlatBuffers.Test/FlatBuffersExampleTests.cs index d5774571c8..d0d326e189 100644 --- a/tests/FlatBuffers.Test/FlatBuffersExampleTests.cs +++ b/tests/FlatBuffers.Test/FlatBuffersExampleTests.cs @@ -1221,7 +1221,7 @@ public void TestVerifyingUnions() Monster.AddTest(fbb, monster_inner.Value); Monster.AddTestType(fbb, Any.Monster); var monster_outer = Monster.EndMonster(fbb); - fbb.Finish(monster_outer.Value); + Monster.FinishMonsterBuffer(fbb, monster_outer); var bytes = fbb.SizedByteArray(); var bytes_to_corrupt_inner_name = fbb.SizedByteArray(); var bytes_to_corrupt_outer_name = fbb.SizedByteArray();