Stader Labs - NoamYakov's results

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderConfig.sol#L176-L183

Vulnerability details

Impact

If the StagerConfig.updateAdmin() function is invoked with StagerConfig's current admin as _admin (possibly because of a human error), it grants it with the admin role (which it already have) and then revoke this role from it.

In this case, the StagerConfig contract is left with no admin forever.

Recommended Mitigation Steps

Revert in StagerConfig.updateAdmin() if _admin == oldAdmin

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L14 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L48 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L62

Vulnerability details

Impact

The Auction contract inherits PausableUpgradeable, and therefore is expected to be pausable. Two of its functions (Auction.createLot() and Auction.addBid()) even use the whenNotPaused modifier, to prevent their invokation when the contract is paused.

For the contract to enter a "paused" state, it must call its internal PausableUpgradeable._pause() function. However, this internal function is never called, and as a result, the contract could not be paused.

Recommended Mitigation Steps

Add admin functions that call PausableUpgradeable._pause() and PausableUpgradeable,_unpause().

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/OperatorRewardsCollector.sol#L16 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/OperatorRewardsCollector.sol#L46

Vulnerability details

Impact

The OperatorRewardsCollector contract inherits PausableUpgradeable, and therefore is expected to be pausable. One of its functions (OperatorRewardsCollector.claim()) even uses the whenNotPaused modifier, to prevent its invokation when the contract is paused.

For the contract to enter a "paused" state, it must call its internal PausableUpgradeable._pause() function. However, this internal function is never called, and as a result, the contract could not be paused.

Recommended Mitigation Steps

Add admin functions that call PausableUpgradeable._pause() and PausableUpgradeable,_unpause().

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderConfig.sol#L102 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderConfig.sol#L176-L183

Vulnerability details

Impact

The initial admin, which is granted with this role on StagerConfig.initialize(), retains its permissions (admin role) after being replaced with another admin via StagerConfig.updateAdmin().

The StagerConfig.updateAdmin() function attempts to revoke the admin role of the admin being replaced, but it fails to do so with the initial admin, because accountsMap[ADMIN] was never set with its address. accountsMap[ADMIN] is updated only via call to StagerConfig.updateAdmin(), and admin role wasn't granted to the initial admin via this function.

If the initial admin was replaced due to security concerns (e.g., it's credentials were compromised), that replacement won't help because the initial admin would retain its permissions.

Recommended Mitigation Steps

Add the following code to StagerConfig.initialize():

accountsMap[ADMIN] = _admin;

Assessed type

Access Control