near-daos / sputnik-dao-contract

Smart contracts for https://app.astrodao.com
https://astrodao.com/
MIT License
108 stars 76 forks source link

Prevent users from overwriting storage deposits of other users #149

Closed ctindogaru closed 2 years ago

ctindogaru commented 2 years ago

The issue was reported by Halborn while doing the audit.

Complete description of the issue: The storage_deposit() function in "sputnik-staking/src/storage_impl.rs" has the condition amount < min_balance && !already_registered on line 26 that would trigger a panic, however the function first attempts to get the account_id value from passed parameters, so an attacker could bypass the condition above to overwrite the balance of an existing registered user as follows:

  1. The victim is registered by themselves or the owner, let's assume a 100 NEAR is attached to the transaction so they have that much balance at the time
  2. The attacker calls the function with the AccountId of the victim and registration_only set to "false" and attaches some NEAR to the transaction
  3. The condition on line 26 fails since the AccountId passed by the attacker is registered
  4. internal_register_user() on line 44 executes and overwrites the existing user registration entry