You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "gty404 (via GitHub)" <gi...@apache.org> on 2026/10/15 08:50:34 UTC
[PR] fix(avro): key projection cache on Avro node to avoid OOB read [iceberg-cpp]
gty404 opened a new pull request, #1022:
URL: https://github.com/apache/iceberg-cpp/pull/1022
## Summary
- `DecodeStructToBuilder` cached the Avro-field-index to projection-index mapping keyed on `projections.data()` only. All empty-struct projections share the same (null) `data()`, so a second empty struct with more Avro fields reused a cached vector sized for the first one and read out of bounds, which could end in a wild-pointer dispatch via `StructBuilder::field_builder`.
- The cache key is now `{projections.data(), avro_node}`. The mapping is built and validated before being cached, and a projected field index outside the Avro record's field count returns `InvalidArgument`.
## Test plan
- [x] New `AvroReaderTest.ProjectEmptyStructsOfDifferentWidths`: file with two struct columns of 1 and 6 fields, both read back as empty structs through the direct decoder
- [x] Test crashes (SIGSEGV) without the fix and passes with it
- [x] Full `avro_test` suite passes (196 tests)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(avro): key projection cache on Avro node to avoid OOB read [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #1022:
URL: https://github.com/apache/iceberg-cpp/pull/1022
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(avro): key projection cache on Avro node to avoid OOB read [iceberg-cpp]
Posted by "shangxinli (via GitHub)" <gi...@apache.org>.
shangxinli commented on code in PR #1022:
URL: https://github.com/apache/iceberg-cpp/pull/1022#discussion_r4284535156
##########
src/iceberg/avro/avro_direct_decoder.cc:
##########
@@ -160,26 +160,32 @@ Status DecodeStructToBuilder(const ::avro::NodePtr& avro_node, ::avro::Decoder&
// Build a map from Avro field index to projection index (cached per struct schema)
// -1 means the field should be skipped
- const FieldProjection* cache_key = projections.data();
+ // Empty projections all share one data() pointer, so the Avro node is part of the key.
+ const DecodeContext::ProjectionCacheKey cache_key{projections.data(), avro_node.get()};
Review Comment:
Without the fix this is an OOB read of uninitialized heap, so the new test only fails reliably under the sanitizer leg. Asserting the cached mapping's size would make it deterministic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(avro): key projection cache on Avro node to avoid OOB read [iceberg-cpp]
Posted by "gty404 (via GitHub)" <gi...@apache.org>.
gty404 commented on code in PR #1022:
URL: https://github.com/apache/iceberg-cpp/pull/1022#discussion_r4290254464
##########
src/iceberg/avro/avro_direct_decoder.cc:
##########
@@ -160,26 +160,32 @@ Status DecodeStructToBuilder(const ::avro::NodePtr& avro_node, ::avro::Decoder&
// Build a map from Avro field index to projection index (cached per struct schema)
// -1 means the field should be skipped
- const FieldProjection* cache_key = projections.data();
+ // Empty projections all share one data() pointer, so the Avro node is part of the key.
+ const DecodeContext::ProjectionCacheKey cache_key{projections.data(), avro_node.get()};
Review Comment:
Good point, thanks. The out-of-bounds read depends on what the heap happens to contain, so the end-to-end test is only reliable under ASAN. I added `DirectDecoderTest.ProjectionCacheKeyedPerAvroNodeForEmptyStructs`, which calls `DecodeAvroToBuilder` directly and asserts the cache has one entry per Avro node with sizes matching each node's `leaves()` (1, 6 and the root's 2). Note that without the fix the decode itself still reads out of bounds before the assertion is reached, so it fails by crash or by assertion depending on the platform, but it can no longer pass silently. Done in a419c023.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(avro): key projection cache on Avro node to avoid OOB read [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #1022:
URL: https://github.com/apache/iceberg-cpp/pull/1022#issuecomment-6331617709
Thanks @gty404 for fixing this and @shangxinli for the quick review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org