Skip to content

fix(drive): replace silent epoch u16 truncation with checked conversion#3293

Merged
QuantumExplorer merged 1 commit into
v3.1-devfrom
fix/epoch-u16-truncation
Mar 15, 2026
Merged

fix(drive): replace silent epoch u16 truncation with checked conversion#3293
QuantumExplorer merged 1 commit into
v3.1-devfrom
fix/epoch-u16-truncation

Conversation

@QuantumExplorer

Copy link
Copy Markdown
Member

Issue Being Fixed or Feature Implemented

Security audit found that mtd.epoch as u16 silently truncates u32 values > 65535. A malicious node returning epoch = 65536 in metadata would cause the client to verify against epoch 0 instead, potentially accepting a proof for the wrong epoch.

What Was Done

  • Replace all as u16 casts in proof verification with try_u32_to_u16() checked conversions that return an error on overflow
  • Fix applies to epoch indices from both request fields and response metadata
  • Also fixes unchecked limit and offset truncations in the same file
  • Removed redundant manual MAX_EPOCH guard in favor of the existing try_u32_to_u16() helper

How Has This Been Tested?

  • Unit tests verify try_u32_to_u16 errors on epoch > u16::MAX (65536, u32::MAX, 100000)
  • Unit tests verify normal epochs still work (0, 1, 42, u16::MAX)
  • Unit tests verify u32_to_u16_opt errors on overflow and works for valid values
  • cargo test -p drive-proof-verifier passes (4 new tests)
  • cargo clippy -p drive-proof-verifier clean
  • cargo fmt --all clean

Breaking Changes

None. This only changes error behavior for values that were previously silently corrupted.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

🤖 Generated with Claude Code

… conversion

Replace all `as u16` casts with `try_u32_to_u16()` checked conversions
in proof verification to prevent silent truncation of epoch indices,
limits, and offsets received from platform nodes.

A malicious node returning epoch=65536 in metadata would cause the
client to verify against epoch 0 due to u32-to-u16 truncation, potentially
accepting a proof for the wrong epoch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@QuantumExplorer has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 4 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 34263861-3cac-467b-8bc9-6c279f1514de

📥 Commits

Reviewing files that changed from the base of the PR and between 658d02d and 419e320.

📒 Files selected for processing (1)
  • packages/rs-drive-proof-verifier/src/proof.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/epoch-u16-truncation
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added this to the v3.1.0 milestone Mar 15, 2026
@QuantumExplorer QuantumExplorer changed the title fix(proof-verifier): replace silent epoch u16 truncation with checked conversion fix(drive): replace silent epoch u16 truncation with checked conversion Mar 15, 2026
@codecov

codecov Bot commented Mar 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.28%. Comparing base (658d02d) to head (419e320).
⚠️ Report is 28 commits behind head on v3.1-dev.

Additional details and impacted files
@@             Coverage Diff              @@
##           v3.1-dev    #3293      +/-   ##
============================================
- Coverage     70.42%   68.28%   -2.15%     
============================================
  Files          3293     3293              
  Lines        262598   262598              
============================================
- Hits         184931   179310    -5621     
- Misses        77667    83288    +5621     
Components Coverage Δ
dpp 56.61% <ø> (-1.82%) ⬇️
drive 74.76% <ø> (-3.44%) ⬇️
drive-abci 81.44% <ø> (-1.49%) ⬇️
sdk 30.34% <ø> (-0.91%) ⬇️
dapi-client 39.08% <ø> (ø)
platform-version ∅ <ø> (∅)
platform-value 39.35% <ø> (ø)
platform-wallet 60.40% <ø> (ø)
drive-proof-verifier ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@QuantumExplorer QuantumExplorer left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Approved

@QuantumExplorer QuantumExplorer merged commit b1307c1 into v3.1-dev Mar 15, 2026
27 of 29 checks passed
@QuantumExplorer QuantumExplorer deleted the fix/epoch-u16-truncation branch March 15, 2026 16:47
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.

1 participant