From ed2415eb7286e29f51eedcdbb81261e3cfc323f1 Mon Sep 17 00:00:00 2001 From: Wouter van Oortmerssen Date: Mon, 13 Aug 2018 09:24:13 -0700 Subject: [PATCH] Fixed use of uoffset_t in verifier could cause wrap around. The verifier must be resilient against any corrupt data, so now using size_t thru-out to ensure any 64-bit offsets can be represented. Also added verification of alignment. Change-Id: I87a22aa6b045c2d83b69b47a47153f2e15ad7e06 Tested: on Linux, also with libfuzzer. --- include/flatbuffers/flatbuffers.h | 38 +++++++++++++++++-------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/include/flatbuffers/flatbuffers.h b/include/flatbuffers/flatbuffers.h index b4d4d1d91..6453c3084 100644 --- a/include/flatbuffers/flatbuffers.h +++ b/include/flatbuffers/flatbuffers.h @@ -1758,7 +1758,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { } // Verify any range within the buffer. - bool Verify(uoffset_t elem, size_t elem_len) const { + bool Verify(size_t elem, size_t elem_len) const { // clang-format off #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE auto upper_bound = elem + elem_len; @@ -1769,19 +1769,23 @@ class Verifier FLATBUFFERS_FINAL_CLASS { return Check(elem_len < size_ && elem <= size_ - elem_len); } + template bool VerifyAlignment(size_t elem) const { + return (elem & (sizeof(T) - 1)) == 0; + } + // Verify a range indicated by sizeof(T). - template bool Verify(uoffset_t elem) const { - return Verify(elem, sizeof(T)); + template bool Verify(size_t elem) const { + return VerifyAlignment(elem) && Verify(elem, sizeof(T)); } // Verify relative to a known-good base pointer. bool Verify(const uint8_t *base, voffset_t elem_off, size_t elem_len) const { - return Verify(static_cast(base - buf_) + elem_off, elem_len); + return Verify(static_cast(base - buf_) + elem_off, elem_len); } template bool Verify(const uint8_t *base, voffset_t elem_off) const { - return Verify(static_cast(base - buf_) + elem_off, sizeof(T)); + return Verify(static_cast(base - buf_) + elem_off, sizeof(T)); } // Verify a pointer (may be NULL) of a table type. @@ -1802,7 +1806,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { // Verify a pointer (may be NULL) to string. bool VerifyString(const String *str) const { - uoffset_t end; + size_t end; return !str || (VerifyVectorOrString(reinterpret_cast(str), 1, &end) && @@ -1812,8 +1816,8 @@ class Verifier FLATBUFFERS_FINAL_CLASS { // Common code between vectors and strings. bool VerifyVectorOrString(const uint8_t *vec, size_t elem_size, - uoffset_t *end = nullptr) const { - auto veco = static_cast(vec - buf_); + size_t *end = nullptr) const { + auto veco = static_cast(vec - buf_); // Check we can read the size field. if (!Verify(veco)) return false; // Check the whole array. If this is a string, the byte past the array @@ -1823,7 +1827,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { if (!Check(size < max_elems)) return false; // Protect against byte_size overflowing. auto byte_size = sizeof(size) + elem_size * size; - if (end) *end = veco + static_cast(byte_size); + if (end) *end = veco + byte_size; return Verify(veco, byte_size); } @@ -1849,19 +1853,19 @@ class Verifier FLATBUFFERS_FINAL_CLASS { bool VerifyTableStart(const uint8_t *table) { // Check the vtable offset. - auto tableo = static_cast(table - buf_); + auto tableo = static_cast(table - buf_); if (!Verify(tableo)) return false; - auto vtableo = static_cast(static_cast(tableo) - - ReadScalar(table)); + // This offset may be signed, but doing the substraction unsigned always + // gives the result we want. + auto vtableo = tableo - static_cast(ReadScalar(table)); // Check the vtable size field, then check vtable fits in its entirety. return VerifyComplexity() && Verify(vtableo) && - (ReadScalar(buf_ + vtableo) & - (sizeof(voffset_t) - 1)) == 0 && + VerifyAlignment(ReadScalar(buf_ + vtableo)) && Verify(vtableo, ReadScalar(buf_ + vtableo)); } template - bool VerifyBufferFromStart(const char *identifier, uoffset_t start) { + bool VerifyBufferFromStart(const char *identifier, size_t start) { if (identifier && (size_ < 2 * sizeof(flatbuffers::uoffset_t) || !BufferHasIdentifier(buf_ + start, identifier))) { @@ -1892,7 +1896,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { VerifyBufferFromStart(identifier, sizeof(uoffset_t)); } - uoffset_t VerifyOffset(uoffset_t start) const { + uoffset_t VerifyOffset(size_t start) const { if (!Verify(start)) return 0; auto o = ReadScalar(buf_ + start); // May not point to itself. @@ -1906,7 +1910,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { } uoffset_t VerifyOffset(const uint8_t *base, voffset_t start) const { - return VerifyOffset(static_cast(base - buf_) + start); + return VerifyOffset(static_cast(base - buf_) + start); } // Called at the start of a table to increase counters measuring data