[Swift] Verify size-prefixed roots from reader position - #9196
Conversation
|
@carrerasdarren-cell Thanks for opening the PR. I was looking at the cpp approach. And it would be better if the swift implementation also follows a similar convention. Where we only skip after we have verified the prefixed value too. It's fine if we duplicate the logic for the verification here getPrefixedSizeCheckedRoot. flatbuffers/include/flatbuffers/verifier.h Line 256 in 81edeb1 flatbuffers/tests/monster_test.cpp Line 609 in 81edeb1 |
Validate the size prefix before advancing ByteBuffer.reader, then use the active reader position consistently for root, file identifier, and returned-object verification. Cover malformed decoy roots, oversized prefixes, short identifiers, and valid prefixed identifiers.
b4a79a7 to
83b8c03
Compare
|
Thanks, updated in |
mustiikhalil
left a comment
There was a problem hiding this comment.
LGTM! amazing work! one of the comments can be disregarded if not needed
|
@carrerasdarren-cell just pinging you so we can get this PR to the finish line |
|
Thanks for the ping. I addressed the remaining wrapping-arithmetic nits in b76d5d5, replied to the optional placement suggestion, and resolved all three review threads. The production Swift module compiles cleanly locally with Swift 6.3.3; the refreshed CI run is now starting. |
|
@carrerasdarren-cell thanks for your contribution |
Summary
ByteBuffer.readerpositionProblem
Both size-prefixed checked-root APIs call
skipPrefix()before delegating togetCheckedRoot. The returned table is constructed from the post-prefix reader position, but verification previously followed a root offset from byte zero and checked a file identifier at byte four.A malformed size-prefixed buffer could therefore place a valid decoy root where the verifier looked while returning a different malformed table from the post-prefix root. The same malformed table is rejected when checked directly, but the prefixed wrappers accepted it. Generated accessors or mutators could then operate outside the buffer bounds despite the checked API succeeding.
Fix
Capture the active reader position once and use it consistently for file-identifier verification, root verification, and table construction. The file-identifier bounds check now accounts for that position without wrapping on short buffers.
Validation
swiftcgit diff --checkpasses