ProtocolDAO lacks a method to take out GGP

[code423n4]

Lines of code

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

Vulnerability details

Impact

ProtocolDAO implementation does not have a method to take out GGP. So it can't handle ggp unless it updates ProtocolDAO

Proof of Concept

recordStakingEnd() will pass the rewards of this reward "If the validator is failing at their duties, their GGP will be slashed and used to compensate the loss to our Liquid Stakers"

At this point slashGGP() will be executed and the GGP will be transferred to "ProtocolDAO"

staking.slashGGP():

    function slashGGP(address stakerAddr, uint256 ggpAmt) public onlySpecificRegisteredContract("MinipoolManager", msg.sender) {
        Vault vault = Vault(getContractAddress("Vault"));
        decreaseGGPStake(stakerAddr, ggpAmt);
        vault.transferToken("ProtocolDAO", ggp, ggpAmt);
    }

But the current ProtocolDAO implementation does not have a method to take out GGP. So it can't handle ggp unless it updates ProtocolDAO

Tools Used

Recommended Mitigation Steps

1.transfer GGP to ClaimProtocolDAO or 2.Similar to ClaimProtocolDAO, add spend method to retrieve GGP

contract ProtocolDAO is Base {
...

+    function spend(
+        address recipientAddress,
+        uint256 amount
+    ) external onlyGuardian {
+        Vault vault = Vault(getContractAddress("Vault"));
+        TokenGGP ggpToken = TokenGGP(getContractAddress("TokenGGP"));
+
+        if (amount == 0 || amount > vault.balanceOfToken("ProtocolDAO", ggpToken)) {
+            revert InvalidAmount();
+        }
+
+        vault.withdrawToken(recipientAddress, ggpToken, amount);
+
+        emit GGPTokensSentByDAOProtocol(address(this), recipientAddress, amount);
+   }

[0xminty]

dupe of #571

[GalloDaSballo]

Making primary because of suggested mitigation

[c4-judge]

GalloDaSballo marked the issue as primary issue

[emersoncloud]

duplicate of https://github.com/code-423n4/2022-12-gogopool-findings/issues/544

[c4-judge]

GalloDaSballo changed the severity to 3 (High Risk)

[GalloDaSballo]

The Warden has shown how, due to a lack of sweep the default contract for fee handling will be unable to retrieve tokens sent to it.

While the issue will definitely would have been discovered fairly early in Prod, the in-scope system makes it clear that the funds would have been sent to ProtocolDAO.sol and would have been lost indefinitely.

For this reason, I believe the finding to be of High Severity

[emersoncloud]

Acknowledged.

Thanks for the report. This is something we're aware of and are not going to fix at the moment.

The funds are transferred to the Vault and the ProtocolDAO contract is upgradeable. Therefore in the future we can upgrade the contract to spend the Vault GGP tokens to return funds to Liquid Stakers.

We expect slashing to be a rare event and might have some manual steps involved in the early days of the protocol to do this process if it occurs

[c4-judge]

GalloDaSballo marked the issue as selected for report