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);