From 09343650e5a7ff1153c1cd65a5f3a3d67226c296 Mon Sep 17 00:00:00 2001 From: pedrobtz Date: Wed, 21 Oct 2026 23:53:47 +0200 Subject: [PATCH] fix(ipc): Zero the padding of footer Blocks Each Block was built on the stack with an initializer and copied into the footer, so its four bytes of padding after metaDataLength were whatever was on the stack. Zero the vector and set each field instead. Co-Authored-By: Claude Opus 5.5 --- src/nanoarrow/ipc/encoder.c | 22 +++++++------- src/nanoarrow/ipc/encoder_test.cc | 49 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/nanoarrow/ipc/encoder.c b/src/nanoarrow/ipc/encoder.c index ca70b244c..2ff855df1 100644 --- a/src/nanoarrow/ipc/encoder.c +++ b/src/nanoarrow/ipc/encoder.c @@ -1226,13 +1226,12 @@ ArrowErrorCode ArrowIpcEncoderEncodeFooter(struct ArrowIpcEncoder* encoder, struct ns(Block)* flatcc_RecordBatch_blocks = ns(Footer_recordBatches_extend(builder, n_blocks)); FLATCC_RETURN_IF_NULL(flatcc_RecordBatch_blocks, error); + // Zero the vector first so that the padding after metaDataLength is + // deterministic, then set each field + memset(flatcc_RecordBatch_blocks, 0, (size_t)n_blocks * sizeof(struct ns(Block))); for (int64_t i = 0; i < n_blocks; i++) { - struct ns(Block) block = { - blocks[i].offset, - blocks[i].metadata_length, - blocks[i].body_length, - }; - flatcc_RecordBatch_blocks[i] = block; + ns(Block_assign)(&flatcc_RecordBatch_blocks[i], blocks[i].offset, + blocks[i].metadata_length, blocks[i].body_length); } FLATCC_RETURN_UNLESS_0(Footer_recordBatches_end(builder), error); @@ -1245,13 +1244,12 @@ ArrowErrorCode ArrowIpcEncoderEncodeFooter(struct ArrowIpcEncoder* encoder, struct ns(Block)* flatcc_dict_blocks = ns(Footer_dictionaries_extend(builder, n_dict_blocks)); FLATCC_RETURN_IF_NULL(flatcc_dict_blocks, error); + // Zero the vector first so that the padding after metaDataLength is + // deterministic, then set each field + memset(flatcc_dict_blocks, 0, (size_t)n_dict_blocks * sizeof(struct ns(Block))); for (int64_t i = 0; i < n_dict_blocks; i++) { - struct ns(Block) block = { - dict_blocks[i].offset, - dict_blocks[i].metadata_length, - dict_blocks[i].body_length, - }; - flatcc_dict_blocks[i] = block; + ns(Block_assign)(&flatcc_dict_blocks[i], dict_blocks[i].offset, + dict_blocks[i].metadata_length, dict_blocks[i].body_length); } FLATCC_RETURN_UNLESS_0(Footer_dictionaries_end(builder), error); diff --git a/src/nanoarrow/ipc/encoder_test.cc b/src/nanoarrow/ipc/encoder_test.cc index fa6b37b3a..eab33afb8 100644 --- a/src/nanoarrow/ipc/encoder_test.cc +++ b/src/nanoarrow/ipc/encoder_test.cc @@ -108,6 +108,55 @@ TEST(NanoarrowIpcTest, NanoarrowIpcFooterEncoding) { EXPECT_GT(footer_buffer->size_bytes, raw_schema_buffer->size_bytes); } +// Fill a stretch of the stack so that uninitialized stack bytes used by a +// subsequent call are not zero by chance +#if defined(_MSC_VER) +__declspec(noinline) +#else +__attribute__((noinline)) +#endif + static void FillStack(uint8_t value) { + volatile uint8_t bytes[16384]; + for (size_t i = 0; i < sizeof(bytes); i++) { + bytes[i] = value; + } +} + +static std::vector EncodeFooterWithBlocks(uint8_t stack_fill) { + nanoarrow::ipc::UniqueEncoder encoder; + EXPECT_EQ(ArrowIpcEncoderInit(encoder.get()), NANOARROW_OK); + + nanoarrow::ipc::UniqueFooter footer; + EXPECT_EQ(ArrowSchemaInitFromType(&footer->schema, NANOARROW_TYPE_STRUCT), + NANOARROW_OK); + struct ArrowIpcFileBlock block = {8, 120, 256}; + EXPECT_EQ(ArrowBufferAppend(&footer->record_batch_blocks, &block, sizeof(block)), + NANOARROW_OK); + EXPECT_EQ(ArrowBufferAppend(&footer->dictionary_blocks, &block, sizeof(block)), + NANOARROW_OK); + + struct ArrowError error; + FillStack(stack_fill); + EXPECT_EQ(ArrowIpcEncoderEncodeFooter(encoder.get(), footer.get(), &error), + NANOARROW_OK) + << error.message; + + nanoarrow::UniqueBuffer footer_buffer; + EXPECT_EQ(ArrowIpcEncoderFinalizeBuffer(encoder.get(), /*encapsulate=*/false, + footer_buffer.get()), + NANOARROW_OK); + return std::vector(footer_buffer->data, + footer_buffer->data + footer_buffer->size_bytes); +} + +TEST(NanoarrowIpcTest, NanoarrowIpcFooterEncodingIsDeterministic) { + // A Block has four bytes of padding after metaDataLength, which must be + // written as zeros rather than whatever was on the stack + std::vector zeros = EncodeFooterWithBlocks(0x00); + std::vector ones = EncodeFooterWithBlocks(0xFF); + EXPECT_EQ(zeros, ones); +} + TEST(NanoarrowIpcTest, NanoarrowIpcEncoderRejectsNestedDictionary) { nanoarrow::UniqueSchema schema; ASSERT_EQ(ArrowSchemaInitFromType(schema.get(), NANOARROW_TYPE_STRUCT), NANOARROW_OK);