sherlock-audit / 2024-08-perennial-v2-update-3-judging

4 stars 2 forks source link

volodya - Coordinator fees will not be able to be claimed #24

Open sherlock-admin4 opened 2 months ago

sherlock-admin4 commented 2 months ago

volodya

Medium

Coordinator fees will not be able to be claimed

Summary

The coordinator's address should be passed to the market's claimFee, but it's never going to happen.

Root Cause

As we can see from code below account == coordinator to get coordinator fees - newGlobal.riskFee

    function claimFee(address account) external onlyOperator(account) returns (UFixed6 feeReceived) {
...
        // risk fee
        if (account == coordinator) {
            feeReceived = feeReceived.add(newGlobal.riskFee);
            newGlobal.riskFee = UFixed6Lib.ZERO;
        }
...
    }

packages/perennial/contracts/Market.sol#L309

but coordinator doesn't pass its address to that function

    function claimFee(IMarket market) external {
        if (msg.sender != comptroller) revert NotComptroller();
        market.claimFee(msg.sender);
        market.token().push(comptroller);
    }

extensions/contracts/Coordinator.sol#L38

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Coordinators fees would be lost

PoC

No response

Mitigation

    function claimFee(IMarket market) external {
        if (msg.sender != comptroller) revert NotComptroller();
-        market.claimFee(msg.sender);
+        market.claimFee(address(this));
        market.token().push(comptroller);
    }
sherlock-admin4 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/equilibria-xyz/perennial-v2/pull/446