sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

rvierdiiev - protocolFee can be updated at any time by protocol #193

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

rvierdiiev

medium

protocolFee can be updated at any time by protocol

Summary

protocolFee can be updated at any time by protocol. But it should settle the product first in order to make product coordinator receive fair payment for previous period.

Vulnerability Detail

Perennial protocol has a fee that is taken from each product earnings. https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/collateral/Collateral.sol#L161-L175

    function settleProduct(UFixed18 amount) external onlyProduct {
        (IProduct product, IController controller) = (IProduct(msg.sender), controller());

        address protocolTreasury = controller.treasury();
        address productTreasury = controller.treasury(product);

        UFixed18 protocolFee = amount.mul(controller.protocolFee());
        UFixed18 productFee = amount.sub(protocolFee);

        _products[product].debit(amount);
        fees[protocolTreasury] = fees[protocolTreasury].add(protocolFee);
        fees[productTreasury] = fees[productTreasury].add(productFee);

        emit ProductSettle(product, protocolFee, productFee);
    }

This fee collecting is done inside settle function.

This fee can be changed at any time. But the problem is that settle is not called for each product before the change. As result this change has impact on previous earning of product.

Impact

Part of product earning can be stolen by protocol.

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

Call settle for each product.

arjun-io commented 1 year ago

It is not feasible gas-wise to enumerate every product and settle them when a protocol param is updated.

KenzoAgada commented 1 year ago

Closing issue due to sponsor comment and low impact.