Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The funds will be locked in the **vault** #420

Closed
code423n4 opened this issue Jan 2, 2023 · 3 comments
Closed

The funds will be locked in the **vault** #420

code423n4 opened this issue Jan 2, 2023 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-723 satisfactory satisfies C4 submission criteria; eligible for awards sponsor duplicate Sponsor deemed duplicate

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

the fund of the nodeOP will be locked on the vault

Proof of Concept

Case 01:
After the Multisig has invoked recordStakingEnd() and before the node op invoke withdrawMinipoolFunds() the Multisig can invoke finishFailedMinipoolByMultisig() the fund will be locked on the vault
Please copy the following POC on MinipoolManager.t.sol

	function test_the_lock_fund_on_vault() public {//!@audit-info -this is a POC
		uint256 originalRialtoBalance = address(rialto).balance;
		address lilly = getActorWithTokens("lilly", MAX_AMT, MAX_AMT);
		vm.prank(lilly);

		ggAVAX.depositAVAX{value: MAX_AMT}();
		assertEq(lilly.balance, 0);

		uint256 duration = 2 weeks;
		uint256 depositAmt = 1000 ether;
		uint256 avaxAssignmentRequest = 1000 ether;
		uint256 validationAmt = depositAmt + avaxAssignmentRequest;
		uint128 ggpStakeAmt = 200 ether;

		vm.startPrank(nodeOp);

		ggp.approve(address(staking), ggpStakeAmt);
		staking.stakeGGP(ggpStakeAmt);
		MinipoolManager.Minipool memory mp = createMinipool(depositAmt, avaxAssignmentRequest, duration);

		vm.stopPrank();

		assertEq(vault.balanceOf("MinipoolManager"), depositAmt);

		vm.startPrank(address(rialto));

		minipoolMgr.claimAndInitiateStaking(mp.nodeID);

		assertEq(vault.balanceOf("MinipoolManager"), 0);
		assertEq(address(rialto).balance - originalRialtoBalance, validationAmt);
		// We simulate a random txID here
		bytes32 txID = keccak256("txid");
		minipoolMgr.recordStakingStart(mp.nodeID, txID, block.timestamp);
		uint256 rewards = 10 ether;
		skip(duration);
		//right now rewards are split equally between the node op and user. User provided half the total funds in this test
		minipoolMgr.recordStakingEnd{value: validationAmt + rewards}(mp.nodeID, block.timestamp, 10 ether);
		uint256 commissionFee = (5 ether * 15) / 100;
		//checking the node operators rewards are corrrect
		assertEq(vault.balanceOf("MinipoolManager"), (1005 ether + commissionFee));
		//!now the funds are locked
        minipoolMgr.finishFailedMinipoolByMultisig(mp.nodeID);
		vm.stopPrank();
        
		//!test that the node op can't withdraw the funds they are due
		vm.startPrank(nodeOp);
		vm.expectRevert(MinipoolManager.InvalidStateTransition.selector);
		minipoolMgr.withdrawMinipoolFunds(mp.nodeID);
		vm.stopPrank();
	}

case 02:
After the Multisig has invoke recordStakingError() he can imeditly call finishFailedMinipoolByMultisig() and the fund will be locked on the vault

Please copy the following POC on MinipoolManager.t.sol

	function test_lock_fund_on_vault_from_Error_to_finish() public {//!@audit-info -this is a POC
		uint256 originalRialtoBalance = address(rialto).balance;
		address lilly = getActorWithTokens("lilly", MAX_AMT, MAX_AMT);
		vm.prank(lilly);

		ggAVAX.depositAVAX{value: MAX_AMT}();
		assertEq(lilly.balance, 0);

		uint256 duration = 2 weeks;
		uint256 depositAmt = 1000 ether;
		uint256 avaxAssignmentRequest = 1000 ether;
		uint256 validationAmt = depositAmt + avaxAssignmentRequest;
		uint128 ggpStakeAmt = 200 ether;

		vm.startPrank(nodeOp);

		ggp.approve(address(staking), ggpStakeAmt);
		staking.stakeGGP(ggpStakeAmt);
		MinipoolManager.Minipool memory mp = createMinipool(depositAmt, avaxAssignmentRequest, duration);

		vm.stopPrank();

		assertEq(vault.balanceOf("MinipoolManager"), depositAmt);

		vm.startPrank(address(rialto));

		minipoolMgr.claimAndInitiateStaking(mp.nodeID);

		assertEq(vault.balanceOf("MinipoolManager"), 0);
		assertEq(address(rialto).balance - originalRialtoBalance, validationAmt);
		// We simulate a random txID here
		bytes32 txID = keccak256("txid");
		minipoolMgr.recordStakingStart(mp.nodeID, txID, block.timestamp);

		bytes32 errorCode = "INVALID_NODEID";
		minipoolMgr.recordStakingError{value: validationAmt}(mp.nodeID, errorCode);

		//!now the funds are locked
        minipoolMgr.finishFailedMinipoolByMultisig(mp.nodeID);
        vm.stopPrank();

		//!test that the node op can't withdraw the funds they are due
		vm.startPrank(nodeOp);
		vm.expectRevert(MinipoolManager.InvalidStateTransition.selector);
		minipoolMgr.withdrawMinipoolFunds(mp.nodeID);
	}

Recommended Mitigation Steps

Make sure that the nodeOP can withdraw his funds in case the status is Finished

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jan 2, 2023
code423n4 added a commit that referenced this issue Jan 2, 2023
C4-Staff added a commit that referenced this issue Jan 6, 2023
@0xju1ie
Copy link

0xju1ie commented Jan 17, 2023

duplicate of #723

@0xju1ie 0xju1ie added the sponsor duplicate Sponsor deemed duplicate label Jan 17, 2023
@c4-judge
Copy link
Contributor

GalloDaSballo marked the issue as duplicate of #723

@c4-judge
Copy link
Contributor

c4-judge commented Feb 8, 2023

GalloDaSballo marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-723 satisfactory satisfies C4 submission criteria; eligible for awards sponsor duplicate Sponsor deemed duplicate
Projects
None yet
Development

No branches or pull requests

3 participants