search

code-423n4 / 2021-11-nested-findings

1 stars 1 forks source link

NestedReserve.updateFactory function can be used for stealing all contract funds (rug) #218

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

hyh

Vulnerability details

Impact

Reserve contract holds funds corresponding to NFTs. The funds can be accessed by Factory and an ability to immediately switch Factory gives a malicious Owner a way to instantly stole all user's funds.

Proof of Concept

updateFactory function allows for unlimited and immediate update of the Factory contract, which has control of all locked funds via transfer and withdraw functions.

https://github.com/code-423n4/2021-11-nested/blob/main/contracts/NestedReserve.sol#L64

Recommended Mitigation Steps

Either make factory immutable by allowing changing it only once: Now:

require(_newFactory != address(0), "NestedReserve: INVALID_ADDRESS");

To be:

require(factory == address(0), "NestedReserve: factory is immutable");

Another way, when Factory updates are planned without explicit funds migration, is to Timelock the updateFactory function.

adrien-supizet commented 2 years ago

As mentioned in the readme, those functions will be timelocked.

alcueca commented 2 years ago

Dispute accepted