Skip to content

Guard NextContractId allocation against overflow and reuse #333

@mikewheeleer

Description

@mikewheeleer

Description

create_contract reads DataKey::NextContractId, defaults to 1, and writes id + 1 using unchecked u32 addition. Near u32::MAX this wraps and could collide with an existing DataKey::Contract(id), silently overwriting an active escrow. Add overflow-safe id allocation.

Requirements and context

  • Scoped to TalentTrust escrow Soroban contract (contracts/escrow).
  • Use checked addition for id + 1 and panic with a dedicated overflow error if the id space is exhausted.
  • Optionally assert DataKey::Contract(id) is unset before writing to detect any collision defensively.
  • Invariant: contract ids are strictly monotonic and never reused; allocation fails closed at the boundary.
  • Must be secure, tested, and documented.

Suggested execution

  • Fork the repo and create a branch:
    • git checkout -b security/contract-id-overflow
  • Implement changes:
    • contracts/escrow/src/lib.rs
    • Tests: contracts/escrow/src/test/storage.rs
    • Docs: docs/escrow/state-persistence.md
    • Include rustdoc/NatSpec-style doc comments on public functions
    • Validate security assumptions (auth, overflow, fail-closed state machine, storage TTL, fee accounting)

Test and commit

  • Run tests: cargo test
  • Cover edge cases (unauthorized callers, double release/refund, expired approvals, fee rounding, paused state)
  • Include test output and security notes in the PR

Example commit message

fix(escrow): overflow-safe NextContractId allocation

Guidelines

  • Minimum 95% test coverage on new/changed code
  • Clear documentation
  • Timeframe: 96 hours from assignment

Metadata

Metadata

Type

No fields configured for Task.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions