Smart contract portfolio ID collision: ledger sequence reuse causes silent data overwrite on concurrent creates

[Uchechukwu-Ekezie]

Summary

create_portfolio derives its ID from env.ledger().sequence() as u64, which is the current ledger sequence number — a value shared across the entire blockchain, not unique per transaction:

// contracts/src/lib.rs
let portfolio_id = env.ledger().sequence() as u64; // Convert u32 to u64

Two transactions landing in the same ledger will produce the same portfolio_id. The second persistent().set silently overwrites the first, destroying the earlier user's portfolio without any error.

Impact

• Loss of funds/state: A user's portfolio is overwritten by another user's, breaking all future deposit and rebalance operations for the original owner.
• No error is raised: Both transactions succeed on-chain; the first user has no signal that their portfolio was destroyed.
• Authorization bypass: After overwrite, portfolio.user becomes the second user's address, so the first user can no longer call deposit or execute_rebalance (requires_auth fails), effectively locking them out.

Steps to Reproduce

1. Submit two create_portfolio calls in transactions included in the same ledger.
2. Query both resulting portfolio IDs — they are identical.
3. Fetch the portfolio — it contains the second user's allocations.

Root Cause

Ledger sequence advances once per closed ledger, not once per transaction or per contract invocation. Relying on it as a unique entity key is fundamentally unsafe.

Suggested Fix

Use a monotonically incrementing counter stored in contract instance storage:

let counter: u64 = env.storage().instance().get(&DataKey::PortfolioCounter).unwrap_or(0);
let portfolio_id = counter + 1;
env.storage().instance().set(&DataKey::PortfolioCounter, &portfolio_id);

Alternatively, derive the ID from a hash of (user, ledger_sequence, tx_hash) if Soroban exposes transaction-level entropy.

References

• contracts/src/lib.rs — create_portfolio function
• contracts/src/types.rs — DataKey::Portfolio(u64) key

Severity: Critical — silent data loss on concurrent portfolio creation

[2TheMoom]

This is precisely the class of vulnerability I look for in smart contract audits. I recently documented a silent asset-lock in the Miden Protocol AggLayer bridge (hardcoded parameter, no error raised, funds permanently locked) and two drain findings in Relay Protocol's periphery contracts -- both involving incorrect assumptions about unique identifiers and authorization boundaries.

Ledger sequence reuse as an entity key is a well-understood Soroban antipattern. The fix is straightforward: replace env.ledger().sequence() as u64 with a monotonically incrementing counter in instance storage using DataKey::PortfolioCounter. I'll also add a test that submits two create_portfolio calls with the same ledger sequence mock to verify the collision no longer occurs. PR within 5-6hrs