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

Slashed GGP tokens are sent to a wrong contract and trapped #571

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

Slashed GGP tokens are sent to a wrong contract and trapped #571

code423n4 opened this issue Jan 3, 2023 · 2 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-532 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

Slashed GGP tokens are trapped in the ProtocolDAO contract and liquid stakers are not compensated.

Proof of Concept

The protocol has a mechanism to slash GGP tokens when the nodes did not operate well (uptime less than 80%).
But looking at the function slashGGP(), the slashed GGP tokens are transferred to the ProtocolDAO contract and there is no way to transfer/spend those.
I believe it is supposed to be sent to ClaimProtocolDAO contract, not the ProtocolDAO, so that it can be spent later.
Furthermore, I don't see any mechanism to compensate the victim liquid stakers.
I believe the protocol intends to exchange the slashed GGP tokens into AVAX and put into the TokenggAVAX so that liquid stakers can claim.

Staking.sol
379: 	function slashGGP(address stakerAddr, uint256 ggpAmt) public onlySpecificRegisteredContract("MinipoolManager", msg.sender) {
380: 		Vault vault = Vault(getContractAddress("Vault"));
381: 		decreaseGGPStake(stakerAddr, ggpAmt);
382: 		vault.transferToken("ProtocolDAO", ggp, ggpAmt);//@audit wrong contract?
383: 	}

Tools Used

Manual Review

Recommended Mitigation Steps

  • Transfer the slashed GGP tokens to ClaimProtocolDAO contract so that the protocol can spend later.
  • Implement a mechanism to compensate the liquid stakers when the node operator's GGP collateral is slashed.
@c4-judge
Copy link
Contributor

c4-judge commented Jan 9, 2023

GalloDaSballo marked the issue as duplicate of #532

@c4-judge c4-judge closed this as completed Jan 9, 2023
@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Feb 9, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Feb 9, 2023

GalloDaSballo marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-532 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants