Skip to content

fix(runtime): guard encode_index_pairs against u32 length truncation#2990

Open
chefsale wants to merge 1 commit into
masterfrom
claude/encode-index-pairs-overflow-6buclw
Open

fix(runtime): guard encode_index_pairs against u32 length truncation#2990
chefsale wants to merge 1 commit into
masterfrom
claude/encode-index-pairs-overflow-6buclw

Conversation

@chefsale

Copy link
Copy Markdown
Member

Replace len() as u32 casts with u32::try_from(...).map_err(IntegerOverflow)
so a future storage-limit bump that allows lengths past u32::MAX surfaces an
error instead of silently corrupting the length prefixes in the wire format.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_016Ess2FvLzJZHzQu9tSsjHJ

Replace `len() as u32` casts with `u32::try_from(...).map_err(IntegerOverflow)`
so a future storage-limit bump that allows lengths past u32::MAX surfaces an
error instead of silently corrupting the length prefixes in the wire format.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Ess2FvLzJZHzQu9tSsjHJ

@meroreviewer meroreviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 MeroReviewer

Reviewed by 1 agents | Quality score: 32% | Review time: 29.3s


💡 Suggestion (1)

1. Missing capacity pre-allocation in encode_index_pairs

File: crates/runtime/src/logic/host_functions/storage.rs (line 590-608) | Consensus: 1/1 agents ✓

The out Vec is created with Vec::new() (zero capacity) and then grown incrementally via multiple extend_from_slice calls inside a loop. For large result sets this causes repeated reallocations. The total byte size can be computed upfront: 4 (count) + per pair: 4 + key.len() + 4 + value.len(). Pre-allocating with Vec::with_capacity(total) would eliminate all reallocations.

Suggested fix:

Compute total = 4 + pairs.iter().map(|(k,v)| 4 + k.len() + 4 + v.len()).sum::<usize>() before creating `out`, then use Vec::with_capacity(total).

Found by: security-reviewer


🤖 Generated by MeroReviewer | Review ID: review-64b15972

@meroreviewer

meroreviewer Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Documentation Review

The following documentation may need updates based on the changes in this PR:

  • 🟡 architecture/app-lifecycle.html: Files matching crates/runtime/** were changed but architecture/app-lifecycle.html was not updated (per source_to_docs_mapping).
  • 🟡 architecture/crates/runtime.html: Files matching crates/runtime/** were changed but architecture/crates/runtime.html was not updated (per source_to_docs_mapping).

@meroreviewer meroreviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 AI Code Reviewer

Reviewed by 1 agents | Quality score: 32% | Review time: 97.6s


💡 Suggestion (1)

1. Missing capacity pre-allocation in encode_index_pairs

File: crates/runtime/src/logic/host_functions/storage.rs (line 592-607) | Consensus: 1/1 agents ✓

The output Vec is created with Vec::new() (zero capacity) and then grown incrementally via extend_from_slice. For a scan that returns N pairs, this causes O(log N) reallocations. The total byte size can be computed upfront: 4 (count) + N * (4 + key_len + 4 + value_len). Pre-allocating with Vec::with_capacity(total) would eliminate all reallocations.

Suggested fix:

Compute `let capacity = 4 + pairs.iter().map(|(k,v)| 4 + k.len() + 4 + v.len()).sum::<usize>(); let mut out = Vec::with_capacity(capacity);` before writing the count prefix.

Found by: security-reviewer


🤖 Generated by AI Code Reviewer | Review ID: review-c021f2b2

@chefsale chefsale changed the title Fix unchecked u32 truncation in encode_index_pairs fix(runtime): guard encode_index_pairs against u32 length truncation Jun 27, 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