Skip to content

Commit

Permalink
Add note on verifying misaligned empty vectors
Browse files Browse the repository at this point in the history
  • Loading branch information
mikkelfj committed Aug 18, 2024
1 parent 86b2f78 commit 4b21380
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
25 changes: 12 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1163,19 +1163,18 @@ sufficient, but for example standard 32-bit Windows only allocates on an
8-byte boundary and can break the monster schema because it has 16-byte
aligned fields.

NOTE: as Aug 2024 it has been discovered that C++ writer code has been
aligning empty vectors to size 4 (for the size field) even if elements
where larger, like the double type. This causes the flatcc verifier to
(correctly) reject these vectors because it would result in an invalid
C pointer type on some architectures. However, because this has been
in effect for over 10 years, the consensus is to have verifiers tolerate
this behaviour even if C++ will eventually fix this issue. As of now
flatcc verifier remains as is but should eventually be updated with
compile time flag to allow this behaviour. It is expected that the
issues will have no practical implications on any readers in any language
but in principle it could least to undefined behaviour in C and C++
due to incorrect pointer alignment. Thus, readers should be OK, but
verifiers could report errors where it should tolerate the misalignment.
NOTE: as of August 2024 it has been discovered that C++ writer code has been
aligning empty vectors to the size field only, even if elements where larger,
like the double type of size 8. This would cause the FlatCC verifier to
(correctly) reject these vectors because it would result in an invalid C pointer
type on some architectures. However, because this has been in effect for over
10 years, the consensus is to have verifiers tolerate this behaviour even if
C++ will eventually fix this issue. The FlatCC verifier will be updated to
accept such buffers by default with a compile time flag to enforce the strict
behaviour as well. In principle the misaligned vectors can lead potentially
lead to undefined behaviour in agressively optimized C compilers. As of now
it appears to be safe to read such buffers on common platforms and it is
preferably to avoid additional runtime reader overhead to deal with this.
For more, see [FlatCC #287], [Google Flatbuffers #8374].


Expand Down
12 changes: 12 additions & 0 deletions doc/binary-format.md
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,18 @@ Note that it is not entirely trivial to check vector lengths because the
element size must be mulplied by the stored element count. For large
elements this can lead to overflows.

NOTE: as of August 2024 it was discovered that a decode long bug in the
C++ builder could result in misaligned vectors when the the length
of the vector was zero and the alignment size larger than the header
fields alignment. In praxis this has not cause any known issues except
that the FlatCC verifier has rejected such buffers. However, it is
problemetatic because it can result in pointers that are not correctly
aligned in C/C++, which is undefined behaviour. Because there are too
presumably too many comprimised buffers in the wild at this point,
combined with the fact that they are currently mostly harmless,
it it recommended to have an option to tolerate misaligned zero-length
vectors as long as the length field is still properly aligned.


## Risks

Expand Down

0 comments on commit 4b21380

Please sign in to comment.