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

1 stars 0 forks source link

GGP can be staked even if contract paused #858

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/b918009eb949705992cf21e4d516c469dbae223b/contracts/contract/Staking.sol#L319-L323 https://github.com/code-423n4/2022-12-gogopool/blob/b918009eb949705992cf21e4d516c469dbae223b/contracts/contract/Staking.sol#L327-L332

Vulnerability details

GGP can be staked even if contract paused

Summary

As can be seen in stakeGGP, it's not expect to allow users to stake when contract is paused. This expected limitation when paused, can be bypassed by restaking GGP

Proof of Concept

Staking.sol#stakeGGP

    /// @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);
    }

stakeGGP doesn't allow to stake only when paused contract, however, rewards can be staked because of missing whenNotPaused modifier in restakeGGP

Staking.sol#restakeGGP

    /// @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);
    }

Mitigation steps

Add the missing modifier 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
+ whenNotPaused
    onlySpecificRegisteredContract("ClaimNodeOp", msg.sender) {
        // Transfer GGP tokens from the ClaimNodeOp contract to this contract
        ggp.safeTransferFrom(msg.sender, address(this), amount);
        _stakeGGP(stakerAddr, amount);
    }
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