code423n4 commented 1 year ago

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 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/SocializingPool.sol#L21 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L17

Vulnerability details

Impact

The Auction contract appears to be implementing PausableUpgradeable from OpenZeppelin Contracts, which includes functions to pause and unpause the contract. These functions are essential to manage situations where there might be a need to halt activities due to security concerns or bug detection. However, the current implementation does not expose these pause and unpause methods, making it impossible to halt operations even in case of emergencies. This could potentially expose the contract to risks, especially if a vulnerability is detected after deployment and the contract needs to be paused for mitigation.

Similar issue occurs with the OperatorRewardsCollector and the claim function, this contract doesn't expose a pause and unpause functions.

Similar issue occurs with the SocializingPool and the claim function, this contract doesn't expose a pause and unpause functions.

Similar issues occur with the OperatorRewardsCollector and the claim function, the SocializingPool and the claim function, and the StaderOracle with the functions submitExchangeRateData, updateERFromPORFeed, closeERInspectionMode, disableERInspectionMode, submitSocializingRewardsMerkleRoot, submitValidatorStats, submitWithdrawnValidators, and submitMissedAttestationPenalties. These contracts also don't expose pause and unpause functions.

Proof of Concept

In the Auction contract, whenNotPaused modifier is used for createLot and addBid functions, indicating that it should have the ability to be paused. But the methods to execute pause and unpause operations are missing in the contract.

Recommended Mitigation Steps

Implement and expose pause and unpause functions in the Auction contract. The functions should be accessible only to the roles that have been granted appropriate permissions to avoid misuse. Here is a sample code using OpenZeppelin's PausableUpgradeable contract:

function pause() public virtual {
    require(hasRole(PAUSER_ROLE, msg.sender), "Auction: must have pauser role to pause");
    _pause();
}

function unpause() public virtual {
    require(hasRole(PAUSER_ROLE, msg.sender), "Auction: must have pauser role to unpause");
    _unpause();
}

In the above code, PAUSER_ROLE is a role that should be defined in the AccessControlUpgradeable and assigned to the addresses which are authorized to pause and unpause the contract.

Assessed type

Other

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #383