diff --git a/CHANGELOG.md b/CHANGELOG.md index 552c9600..68fa3f2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ - Fix missing runtime check for building too large tables (#235). - Fix alignment of large objects created outside root buffer (#127). - Pad top level buffer end to largest object in buffer +- Add `_with_size` verifiers to enable verification of size prefixed buffers + that are aligned above 4 bytes (sizeof(uoffset_t)) (#210, PR #213). + Also fixes a (uoffset_t) cast that would truncate very large verifier `bufsiz` + arguments above 4GB before the verifier checked for argument overflow + (not expected to be a practical problem, but could be abused). ## [0.6.1] diff --git a/include/flatcc/flatcc_verifier.h b/include/flatcc/flatcc_verifier.h index 7e0d2966..eb19b64b 100644 --- a/include/flatcc/flatcc_verifier.h +++ b/include/flatcc/flatcc_verifier.h @@ -90,9 +90,11 @@ extern "C" { XX(union_element_present_with_type_NONE, "union element present with type NONE")\ XX(union_vector_length_mismatch, "union type and table vectors have different lengths")\ XX(union_vector_verification_not_supported, "union vector verification not supported")\ + XX(runtime_buffer_size_less_than_size_field, "runtime buffer size less than buffer headers size field")\ XX(not_supported, "not supported") + enum flatcc_verify_error_no { #define XX(no, str) flatcc_verify_error_##no, FLATCC_VERIFY_ERROR_MAP(XX) @@ -163,18 +165,34 @@ typedef int flatcc_union_verifier_f(flatcc_union_verifier_descriptor_t *ud); * The buffer must at least be aligned to uoffset_t on systems that * require aligned memory addresses. The buffer pointers alignment is * not significant to internal verification of the buffer. + * + * The `_with_size` variant handles size prefixed buffers which aligns slighly differently + * due to the size prefix, notably for buffers with alignment above the uoffset_t type. */ int flatcc_verify_struct_as_root(const void *buf, size_t bufsiz, const char *fid, size_t size, uint16_t align); +int flatcc_verify_struct_as_root_with_size(const void *buf, size_t bufsiz, const char *fid, + size_t size, uint16_t align); + int flatcc_verify_struct_as_typed_root(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, size_t size, uint16_t align); +int flatcc_verify_struct_as_typed_root_with_size(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, + size_t size, uint16_t align); + int flatcc_verify_table_as_root(const void *buf, size_t bufsiz, const char *fid, flatcc_table_verifier_f *root_tvf); +int flatcc_verify_table_as_root_with_size(const void *buf, size_t bufsiz, const char *fid, + flatcc_table_verifier_f *root_tvf); + int flatcc_verify_table_as_typed_root(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, flatcc_table_verifier_f *root_tvf); + +int flatcc_verify_table_as_typed_root_with_size(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, + flatcc_table_verifier_f *root_tvf); + /* * The buffer header is verified by any of the `_as_root` verifiers, but * this function may be used as a quick sanity check. @@ -183,6 +201,16 @@ int flatcc_verify_buffer_header(const void *buf, size_t bufsiz, const char *fid) int flatcc_verify_typed_buffer_header(const void *buf, size_t bufsiz, flatbuffers_thash_t type_hash); +/* + * Verifies size prefixed buffer headers. The `bufsiz` argument is a pointer that will be updated + * with the size stored in the buffer header iff it is no larger than the input argument, and + * otherwise the verifer fails. The updated size field adds sizeof(flatbuffers_uoffset_t) to the size + * to be compatible with bufsiz since the header size field does not include itself. + */ +int flatcc_verify_buffer_header_with_size(const void *buf, size_t *bufsiz, const char *fid); + +int flatcc_verify_typed_buffer_header_with_size(const void *buf, size_t *bufsiz, flatbuffers_thash_t type_hash); + /* * The following functions are typically called by a generated table * verifier function. diff --git a/src/compiler/codegen_c_verifier.c b/src/compiler/codegen_c_verifier.c index 9b1a0486..7bedfa71 100644 --- a/src/compiler/codegen_c_verifier.c +++ b/src/compiler/codegen_c_verifier.c @@ -215,18 +215,34 @@ static int gen_table_verifier(fb_output_t *out, fb_compound_type_t *ct) "static inline int %s_verify_as_root(const void *buf, size_t bufsiz)\n" "{\n return flatcc_verify_table_as_root(buf, bufsiz, %s_identifier, &%s_verify_table);\n}\n\n", snt.text, snt.text, snt.text); + fprintf(out->fp, + "static inline int %s_verify_as_root_with_size(const void *buf, size_t bufsiz)\n" + "{\n return flatcc_verify_table_as_root_with_size(buf, bufsiz, %s_identifier, &%s_verify_table);\n}\n\n", + snt.text, snt.text, snt.text); fprintf(out->fp, "static inline int %s_verify_as_typed_root(const void *buf, size_t bufsiz)\n" "{\n return flatcc_verify_table_as_root(buf, bufsiz, %s_type_identifier, &%s_verify_table);\n}\n\n", snt.text, snt.text, snt.text); + fprintf(out->fp, + "static inline int %s_verify_as_typed_root_with_size(const void *buf, size_t bufsiz)\n" + "{\n return flatcc_verify_table_as_root_with_size(buf, bufsiz, %s_type_identifier, &%s_verify_table);\n}\n\n", + snt.text, snt.text, snt.text); fprintf(out->fp, "static inline int %s_verify_as_root_with_identifier(const void *buf, size_t bufsiz, const char *fid)\n" "{\n return flatcc_verify_table_as_root(buf, bufsiz, fid, &%s_verify_table);\n}\n\n", snt.text, snt.text); + fprintf(out->fp, + "static inline int %s_verify_as_root_with_identifier_and_size(const void *buf, size_t bufsiz, const char *fid)\n" + "{\n return flatcc_verify_table_as_root_with_size(buf, bufsiz, fid, &%s_verify_table);\n}\n\n", + snt.text, snt.text); fprintf(out->fp, "static inline int %s_verify_as_root_with_type_hash(const void *buf, size_t bufsiz, %sthash_t thash)\n" "{\n return flatcc_verify_table_as_typed_root(buf, bufsiz, thash, &%s_verify_table);\n}\n\n", snt.text, nsc, snt.text); + fprintf(out->fp, + "static inline int %s_verify_as_root_with_type_hash_and_size(const void *buf, size_t bufsiz, %sthash_t thash)\n" + "{\n return flatcc_verify_table_as_typed_root_with_size(buf, bufsiz, thash, &%s_verify_table);\n}\n\n", + snt.text, nsc, snt.text); return 0; } @@ -241,18 +257,34 @@ static int gen_struct_verifier(fb_output_t *out, fb_compound_type_t *ct) "static inline int %s_verify_as_root(const void *buf, size_t bufsiz)\n" "{\n return flatcc_verify_struct_as_root(buf, bufsiz, %s_identifier, %"PRIu64", %"PRIu16");\n}\n\n", snt.text, snt.text, ct->size, ct->align); + fprintf(out->fp, + "static inline int %s_verify_as_root_with_size(const void *buf, size_t bufsiz)\n" + "{\n return flatcc_verify_struct_as_root_with_size(buf, bufsiz, %s_identifier, %"PRIu64", %"PRIu16");\n}\n\n", + snt.text, snt.text, ct->size, ct->align); fprintf(out->fp, "static inline int %s_verify_as_typed_root(const void *buf, size_t bufsiz)\n" "{\n return flatcc_verify_struct_as_typed_root(buf, bufsiz, %s_type_hash, %"PRIu64", %"PRIu16");\n}\n\n", snt.text, snt.text, ct->size, ct->align); + fprintf(out->fp, + "static inline int %s_verify_as_typed_root_with_size(const void *buf, size_t bufsiz)\n" + "{\n return flatcc_verify_struct_as_typed_root_with_size(buf, bufsiz, %s_type_hash, %"PRIu64", %"PRIu16");\n}\n\n", + snt.text, snt.text, ct->size, ct->align); fprintf(out->fp, "static inline int %s_verify_as_root_with_type_hash(const void *buf, size_t bufsiz, %sthash_t thash)\n" "{\n return flatcc_verify_struct_as_typed_root(buf, bufsiz, thash, %"PRIu64", %"PRIu16");\n}\n\n", snt.text, out->nsc, ct->size, ct->align); + fprintf(out->fp, + "static inline int %s_verify_as_root_with_type_hash_and_size(const void *buf, size_t bufsiz, %sthash_t thash)\n" + "{\n return flatcc_verify_struct_as_typed_root_with_size(buf, bufsiz, thash, %"PRIu64", %"PRIu16");\n}\n\n", + snt.text, out->nsc, ct->size, ct->align); fprintf(out->fp, "static inline int %s_verify_as_root_with_identifier(const void *buf, size_t bufsiz, const char *fid)\n" "{\n return flatcc_verify_struct_as_root(buf, bufsiz, fid, %"PRIu64", %"PRIu16");\n}\n\n", snt.text, ct->size, ct->align); + fprintf(out->fp, + "static inline int %s_verify_as_root_with_identifier_and_size(const void *buf, size_t bufsiz, const char *fid)\n" + "{\n return flatcc_verify_struct_as_root_with_size(buf, bufsiz, fid, %"PRIu64", %"PRIu16");\n}\n\n", + snt.text, ct->size, ct->align); return 0; } diff --git a/src/runtime/verifier.c b/src/runtime/verifier.c index 9c43bf61..fb9a67f7 100644 --- a/src/runtime/verifier.c +++ b/src/runtime/verifier.c @@ -470,6 +470,29 @@ int flatcc_verify_buffer_header(const void *buf, size_t bufsiz, const char *fid) return flatcc_verify_ok; } +int flatcc_verify_buffer_header_with_size(const void *buf, size_t *bufsiz, const char *fid) +{ + thash_t id, id2; + size_t size_field; + + verify_runtime(!(((size_t)buf) & (offset_size - 1)), flatcc_verify_error_runtime_buffer_header_not_aligned); + /* -8 ensures no scalar or offset field size can overflow. */ + verify_runtime(*bufsiz <= FLATBUFFERS_UOFFSET_MAX - 8, flatcc_verify_error_runtime_buffer_size_too_large); + + /* Size field, offset field, optional identifier field that must be read even if not present. */ + verify(*bufsiz >= 2 * offset_size + FLATBUFFERS_IDENTIFIER_SIZE, flatcc_verify_error_buffer_header_too_small); + + size_field = (size_t)read_uoffset(buf, 0); + verify_runtime(size_field <= *bufsiz - offset_size, flatcc_verify_error_runtime_buffer_size_less_than_size_field); + if (fid != 0) { + id2 = read_thash_identifier(fid); + id = read_thash(buf, offset_size); + verify(id2 == 0 || id == id2, flatcc_verify_error_identifier_mismatch); + } + *bufsiz = size_field + offset_size; + return flatcc_verify_ok; +} + int flatcc_verify_typed_buffer_header(const void *buf, size_t bufsiz, flatbuffers_thash_t thash) { thash_t id, id2; @@ -492,30 +515,77 @@ int flatcc_verify_typed_buffer_header(const void *buf, size_t bufsiz, flatbuffer return flatcc_verify_ok; } +int flatcc_verify_typed_buffer_header_with_size(const void *buf, size_t *bufsiz, flatbuffers_thash_t thash) +{ + thash_t id, id2; + size_t size_field; + + verify_runtime(!(((size_t)buf) & (offset_size - 1)), flatcc_verify_error_runtime_buffer_header_not_aligned); + /* -8 ensures no scalar or offset field size can overflow. */ + verify_runtime(*bufsiz <= FLATBUFFERS_UOFFSET_MAX - 8, flatcc_verify_error_runtime_buffer_size_too_large); + + /* Size field, offset field, optional identifier field that must be read even if not present. */ + verify(*bufsiz >= 2 * offset_size + FLATBUFFERS_IDENTIFIER_SIZE, flatcc_verify_error_buffer_header_too_small); + + size_field = (size_t)read_uoffset(buf, 0); + verify_runtime(size_field <= *bufsiz - offset_size, flatcc_verify_error_runtime_buffer_size_less_than_size_field); + if (thash != 0) { + id2 = thash; + id = read_thash(buf, offset_size); + verify(id2 == 0 || id == id2, flatcc_verify_error_identifier_mismatch); + } + *bufsiz = size_field + offset_size; + return flatcc_verify_ok; +} + int flatcc_verify_struct_as_root(const void *buf, size_t bufsiz, const char *fid, size_t size, uint16_t align) { check_result(flatcc_verify_buffer_header(buf, bufsiz, fid)); return verify_struct((uoffset_t)bufsiz, 0, read_uoffset(buf, 0), (uoffset_t)size, align); } +int flatcc_verify_struct_as_root_with_size(const void *buf, size_t bufsiz, const char *fid, size_t size, uint16_t align) +{ + check_result(flatcc_verify_buffer_header_with_size(buf, &bufsiz, fid)); + return verify_struct((uoffset_t)bufsiz, 0, read_uoffset(buf, 0), (uoffset_t)size, align); +} + int flatcc_verify_struct_as_typed_root(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, size_t size, uint16_t align) { check_result(flatcc_verify_typed_buffer_header(buf, bufsiz, thash)); return verify_struct((uoffset_t)bufsiz, 0, read_uoffset(buf, 0), (uoffset_t)size, align); } +int flatcc_verify_struct_as_typed_root_with_size(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, size_t size, uint16_t align) +{ + check_result(flatcc_verify_typed_buffer_header_with_size(buf, &bufsiz, thash)); + return verify_struct((uoffset_t)bufsiz, uoffset_size, read_uoffset(buf, uoffset_size), (uoffset_t)size, align); +} + int flatcc_verify_table_as_root(const void *buf, size_t bufsiz, const char *fid, flatcc_table_verifier_f *tvf) { - check_result(flatcc_verify_buffer_header(buf, (uoffset_t)bufsiz, fid)); + check_result(flatcc_verify_buffer_header(buf, bufsiz, fid)); return verify_table(buf, (uoffset_t)bufsiz, 0, read_uoffset(buf, 0), FLATCC_VERIFIER_MAX_LEVELS, tvf); } +int flatcc_verify_table_as_root_with_size(const void *buf, size_t bufsiz, const char *fid, flatcc_table_verifier_f *tvf) +{ + check_result(flatcc_verify_buffer_header_with_size(buf, &bufsiz, fid)); + return verify_table(buf, (uoffset_t)bufsiz, uoffset_size, read_uoffset(buf, uoffset_size), FLATCC_VERIFIER_MAX_LEVELS, tvf); +} + int flatcc_verify_table_as_typed_root(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, flatcc_table_verifier_f *tvf) { - check_result(flatcc_verify_typed_buffer_header(buf, (uoffset_t)bufsiz, thash)); + check_result(flatcc_verify_typed_buffer_header(buf, bufsiz, thash)); return verify_table(buf, (uoffset_t)bufsiz, 0, read_uoffset(buf, 0), FLATCC_VERIFIER_MAX_LEVELS, tvf); } +int flatcc_verify_table_as_typed_root_with_size(const void *buf, size_t bufsiz, flatbuffers_thash_t thash, flatcc_table_verifier_f *tvf) +{ + check_result(flatcc_verify_typed_buffer_header_with_size(buf, &bufsiz, thash)); + return verify_table(buf, (uoffset_t)bufsiz, uoffset_size, read_uoffset(buf, uoffset_size), FLATCC_VERIFIER_MAX_LEVELS, tvf); +} + int flatcc_verify_struct_as_nested_root(flatcc_table_verifier_descriptor_t *td, voffset_t id, int required, const char *fid, size_t size, uint16_t align) { diff --git a/test/doublevec_test/doublevec_test.c b/test/doublevec_test/doublevec_test.c index a50036a5..5cad9d30 100644 --- a/test/doublevec_test/doublevec_test.c +++ b/test/doublevec_test/doublevec_test.c @@ -54,28 +54,28 @@ int test_doublevec(flatcc_builder_t *B) int test_doublevec_with_size(flatcc_builder_t *B) { - void *buffer, *frame; - size_t size, size2, esize; + void *buffer; + size_t size; int ret; gen_doublevec_with_size(B); - frame = flatcc_builder_finalize_aligned_buffer(B, &size); - hexdump("doublevec table with size", frame, size, stderr); - - buffer = flatbuffers_read_size_prefix(frame, &size2); - esize = size - sizeof(flatbuffers_uoffset_t); - if (size2 != esize) { - printf("Size prefix has unexpected size, got %i, expected %i\n", (int)size2, (int)esize); - return -1; - } - - if ((ret = DoubleVec_verify_as_root(buffer, size))) { + buffer = flatcc_builder_finalize_aligned_buffer(B, &size); + hexdump("doublevec table with size", buffer, size, stderr); + + /* Verifiers `_with_size` were introduced in v0.6.2. Previously verifiers would have to skip the size field and verify + the buffer after the size field, but this can lead to misalignment for fields larger than the uoffset_t type + so the buffer would have to be copied to be copied down to to overwrite the size field for this to work. + The `_with_size` verifier handles this problem better. + The header size field was not part of the original Flatbuffers design, and remains optional. + Note that there is nothing special about vectors, they differentiating part is the double type that + can trigger this issue because it has an 8 byte alignment. */ + if ((ret = DoubleVec_verify_as_root_with_size(buffer, size))) { printf("doublevec buffer failed to verify, got: %s\n", flatcc_verify_error_string(ret)); return -1; } - flatcc_builder_aligned_free(frame); + flatcc_builder_aligned_free(buffer); return ret; }