Lack of Pause and Unpause Functionality in Auction Contract
code423n4 opened this issue · 2 comments
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
Picodes marked the issue as satisfactory