Skip to content

Fix heap-buffer-overflow in Array deserialization#14

Merged
austinhartzheim merged 1 commit intocloudflare:mainfrom
silverfoxy:fix-array-cap-mismatch
Feb 11, 2026
Merged

Fix heap-buffer-overflow in Array deserialization#14
austinhartzheim merged 1 commit intocloudflare:mainfrom
silverfoxy:fix-array-cap-mismatch

Conversation

@silverfoxy
Copy link
Contributor

@silverfoxy silverfoxy commented Feb 10, 2026

This PR fixes a bug where Array::from_vec and Array::From disagreed on capacity calculation, causing heap-buffer-overflow when deserializing with certain serializers (e.g., bincode)
I added simple regression test for the capacity mismatch issue.

The Bug
from_vec used cap = arr.len() while From assumed cap = len.next_power_of_two().

When deserializing 100 elements:

  1. Deserializer creates Vec with capacity 100
  2. from_vec stores pointer with capacity 100
  3. Later, From reconstructs assuming capacity 128
  4. Writing to indices 100-127 overflows the heap buffer
    This affects serializers like bincode that create exact-capacity Vecs. serde_json accidentally masked the bug because push() doubles capacity.

The Fix
from_vec now resizes the Vec to len.next_power_of_two() before forgetting it, ensuring the allocation matches what From expects.

@silverfoxy silverfoxy force-pushed the fix-array-cap-mismatch branch 2 times, most recently from 0bd3661 to e166092 Compare February 10, 2026 22:13
@silverfoxy silverfoxy force-pushed the fix-array-cap-mismatch branch from e166092 to 142b301 Compare February 10, 2026 23:51
/// Returns the representation type of `CardinalityEstimator`.
#[inline]
pub(crate) fn representation(&self) -> Representation<P, W> {
pub(crate) fn representation(&self) -> Representation<'_, P, W> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes clippy happy.

  --> src/estimator.rs:98:34
   |
98 |     pub(crate) fn representation(&self) -> Representation<P, W> {
   |                                  ^^^^^     ^^^^^^^^^^^^^^^^^^^^ the same lifetime is hidden here
   |                                  |
   |                                  the lifetime is elided here
   |
   = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
   = note: `-D mismatched-lifetime-syntaxes` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(mismatched_lifetime_syntaxes)]`
help: use `'_` for type paths
   |
98 |     pub(crate) fn representation(&self) -> Representation<'_, P, W> {

@austinhartzheim austinhartzheim merged commit 83e411a into cloudflare:main Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants