search

code-423n4 / 2022-04-dualityfocus-findings

1 stars 0 forks source link

Transferring admins does not work for CToken #13

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-dualityfocus/blob/f21ef7708c9335ee1996142e2581cb8714a525c9/contracts/compound_rari_fork/CToken.sol#L1379

Vulnerability details

Impact

The CToken implements an _acceptAdmin function that sets the new admin to the pendingAdmin. But CToken does not implement a _setPendingAdmin function to set the pendingAdmin in the first place. Therefore, the _acceptAdmin function is useless and CTokens can never change their admin, breaking the desired functionality.

Recommended Mitigation Steps

Add a _setPendingAdmin function to CToken as in UniTroller or remove the _acceptAdmin function if the admin should never be changed.

0xdramaone commented 2 years ago

We are utilizing CErc20Immutable just as provided in Compound. Admin is not meant to change under immutable. Agreed though for clarity we should either add transferring or remove the accept admin function.

0xdramaone commented 2 years ago

Duplicate of #29