sherlock-audit / 2024-06-makerdao-endgame-judging

1 stars 1 forks source link

panprog - Lockstake vault can be approved to be managed by any manager, not just the owner #65

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

panprog

Medium

Lockstake vault can be approved to be managed by any manager, not just the owner

Summary

Owner of Lockstake vault (Lockstake urn) can approve anyone to manage the vault on his behalf by calling LockstateEngine.hope. The issue is that this function can be called by any address already approved to manage the vault, not just the owner. This can cause the loss of all vault funds for the user if any approved manager turns malicious and approves the other malicious addresses, so that user is either unaware of this or simply can't do anything about it (see below for possible Attack Paths).

Root Cause

User authorization for both normal and management functions are protected by the urnAuth modifier: https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeEngine.sol#L132-L134 which calls _urnAuth function: https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeEngine.sol#L175-L177 This function verifies that caller is either urn owner or urn manager.

Usage of this function for both normal and management functions (hope and nope) means that any manager can approve or disapprove the other managers.

Internal pre-conditions

User approves some address to manage his Lockstake vault, at some point of time this address turns malicious (EOA address stolen/hacked, smart contract manager is hacked etc)

External pre-conditions

None

Attack Path

There are multiple possible scenarios leading to loss of funds. For example:

  1. User approves someone to manage his vault
  2. After some time user withdraws his funds
  3. Sometime after that the manager turns malicious and approves the other address(es) to manage this vault
  4. User, being aware of malicious manager, revokes the managing rights (calling nope)
  5. User deposits funds into his vault again
  6. The other address approved by malicious manager steals all user vault by calling free

Even if the user is aware that malicious manager can approve the other managers, it's still possible that he can't do anything about it, for example:

  1. User approves some smart contract to manage his vault where the smart contract doesn't allow anyone other than user to withdraw funds.
  2. The smart contract is hacked, but still doesn't allow hacker to withdraw funds
  3. The hacked smart contract approves the other address, which then withdraws the funds

Impact

Entire user's Lockstate vault is stolen.

PoC

Not needed

Mitigation

Consider only the vault owner to call hope and nope functions of LockstateEngine.

telome commented 1 month ago

This is intended behaviour. Owners should only hope addresses they trust.