OpenZeppelin / OpenZeppelin/stellar-contracts

🐞 [Bug]: Reassigned recovery target bypasses MaxBalance per-identity accounting

Open
#886 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
95
Forks
68
Avg merge
4d 42m
Merged PRs (30d)
20

Description

What happened?

recover_identity(old, target) moves an identity to a new wallet, but the token balance of old follows later through recover_balance. Between the two calls the registry lets target change identity: remove_identity(target) only checks target's own balances, which are still zero, and add_identity(target, ...) only rejects wallets that were recovery sources. When recover_balance(old, target) then runs, MaxBalance::on_transfer resolves the tombstoned old through target's current identity, so both sides resolve to the new identity and the same-identity early return skips the books.

Sequence, with a MaxBalance cap of 100:

  1. Mint 100 to old, registered under identity A. A's tracked balance is 100.
  2. recover_identity(old, target).
  3. remove_identity(target) passes, since target holds nothing.
  4. add_identity(target, B) passes.
  5. recover_balance(old, target) moves 100 tokens. MaxBalance is a no-op.
  6. Mint 100 to another wallet of B passes. B holds 200 against a cap of 100, and A carries a ghost 100 that can never be debited.

Every step is an operator action, but the result is unrecoverable accounting drift, and A's tokens end up in a wallet of a different investor without passing a compliance check.

Affected code: remove_identity and add_identity in packages/tokens/src/rwa/identity_verification/identity_registry_storage/storage.rs, on_transfer in packages/tokens/src/rwa/compliance/modules/max_balance/storage.rs.

Surfaced by the OpenZeppelin platform scan on main (035da1a8) as L-01.

Expected behavior

The registry pins the recovered identity to target until old has been drained: remove_identity(target) is rejected while any linked token still reports a non-zero balance for old. The same guard applies to recover_identity(target, ...), so a chained recovery cannot strand old's tokens.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with remove_identity and add_identity in packages/tokens/src/rwa/identity_verification/identity_registry_storage/storage.rs, then trace recover_identity and recover_balance into on_transfer in packages/tokens/src/rwa/compliance/modules/max_balance/storage.rs. Reproduce the documented reassignment sequence and verify that recovery targets cannot be changed while the old identity has linked token balances, including chained recovery.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
blockchain, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.