Skip to content

fix(pagination): clamp page to prevent u32 overflow - #60

Open
Emelie-Dev wants to merge 2 commits into
StellarSend:mainfrom
Emelie-Dev:fix/issue-transaction-pagination-overflow
Open

fix(pagination): clamp page to prevent u32 overflow#60
Emelie-Dev wants to merge 2 commits into
StellarSend:mainfrom
Emelie-Dev:fix/issue-transaction-pagination-overflow

Conversation

@Emelie-Dev

Copy link
Copy Markdown

Description

Fixes an issue where providing extremely large values for the page query parameter (such as u32::MAX) caused a silent wraparound overflow in release builds and a panic in debug builds when computing the pagination offset.

This PR prevents this behavior by checking for u32 arithmetic overflow, returning a clean validation error if the parsed page results in an uncomputable offset, and explicitly enforces integer overflow checks system-wide in release builds.

Changes

  • Checked Pagination Math: Updated the offset computation in TransactionService::list_for_user to use checked_sub and checked_mul. This explicitly yields an AppError::Validation if an overflow occurs, resulting in a predictable 422 Unprocessable Entity response.
  • Test Coverage: Added compute_offset unit tests in src/services/transaction.rs covering typical inputs, extremely high values near the u32 boundary, and out-of-bounds page parameters to verify proper error propagation.
  • Release Overflow Protection: Enabled overflow-checks = true in the [profile.release] section of Cargo.toml. This acts as defense in depth against this class of bug appearing anywhere else in the application logic.

Validation and Testing

  • Offset logic behaves predictably at upper boundaries (tested page = 1, page = 42949673, and page = u32::MAX).
  • The application will now return a Validation error properly when passing bounds, terminating execution safely.
  • Code passes local cargo fmt and cargo check.

Related Issues

  • Resolves: TransactionListParams.page/per_page unchecked u32 multiplication overflow issue

closes #57

@abayomicornelius

Copy link
Copy Markdown
Contributor

resolve conflicts

@Emelie-Dev

Copy link
Copy Markdown
Author

@abayomicornelius

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.

TransactionListParams.page has no upper bound — (page - 1) * per_page can overflow u32 (panic in debug, silent wraparound in release)

2 participants