code-423n4 / 2022-12-gogopool-findings

1 stars 0 forks source link

Missing WhenNotPaused modifier for restakeGGP in Staking.sol #351

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/Staking.sol#L328 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ClaimNodeOp.sol#L89

Vulnerability details

Impact

Missing WhenNotPaused modifier for restakeGGP in Staking.sol

Proof of Concept

The admin can pause a contract in urgent sitation or the governance can pause a contract as they see fit.

/// @dev Verify contract is not paused
modifier whenNotPaused() {
    string memory contractName = getContractName(address(this));
    if (getBool(keccak256(abi.encodePacked("contract.paused", contractName)))) {
        revert ContractPaused();
    }
    _;
}

As we can see in the code, when the contract is paused, the stakeGGP is blocked in Staking.sol

/// @notice Accept a GGP stake
/// @param amount The amount of GGP being staked
function stakeGGP(uint256 amount) external whenNotPaused {
    // Transfer GGP tokens from staker to this contract
    ggp.safeTransferFrom(msg.sender, address(this), amount);
    _stakeGGP(msg.sender, amount);
}

but the restakeGGP function is missing WhenNotPaused.

/// @notice Convenience function to allow for restaking claimed GGP rewards
/// @param stakerAddr The C-chain address of a GGP staker in the protocol
/// @param amount The amount of GGP being staked
function restakeGGP(address stakerAddr, uint256 amount) public onlySpecificRegisteredContract("ClaimNodeOp", msg.sender) {
    // Transfer GGP tokens from the ClaimNodeOp contract to this contract
    ggp.safeTransferFrom(msg.sender, address(this), amount);
    _stakeGGP(stakerAddr, amount);
}

Use can bypass the WhenNotPaused modifier by calling ClaimNodeOp#claimAndRestake

    /// @notice Claim GGP and automatically restake the remaining unclaimed rewards
    /// @param claimAmt The amount of GGP the staker would like to withdraw from the protocol
    function claimAndRestake(uint256 claimAmt) external {
        Staking staking = Staking(getContractAddress("Staking"));
        uint256 ggpRewards = staking.getGGPRewards(msg.sender);
        if (ggpRewards == 0) {
            revert NoRewardsToClaim();
        }
        if (claimAmt > ggpRewards) {
            revert InvalidAmount();
        }

        staking.decreaseGGPRewards(msg.sender, ggpRewards);

        Vault vault = Vault(getContractAddress("Vault"));
        uint256 restakeAmt = ggpRewards - claimAmt;
        if (restakeAmt > 0) {
            vault.withdrawToken(address(this), ggp, restakeAmt);
            ggp.approve(address(staking), restakeAmt);
            staking.restakeGGP(msg.sender, restakeAmt);
        }

        if (claimAmt > 0) {
            vault.withdrawToken(msg.sender, ggp, claimAmt);
        }

        emit GGPRewardsClaimed(msg.sender, claimAmt);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the project add the WhenNotPaused modifier to restakeGGP to not let user bypass the paused state

GalloDaSballo commented 1 year ago

Pretty similar to others but I think Med is the most appropriate submission (will Judge severity later)

c4-judge commented 1 year ago

GalloDaSballo marked the issue as primary issue

emersoncloud commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-12-gogopool-findings/issues/673

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #673

c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory