sherlock-audit / 2024-01-looksrare-judging

3 stars 0 forks source link

ast3ros - The protocol fee can be wrongly calculated. #143

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

ast3ros

medium

The protocol fee can be wrongly calculated.

Summary

Changes in the protocol fee discount rate can lead to users paying a lower fee than originally intended, resulting in potential revenue loss for the protocol.

Vulnerability Detail

The issue arises during the fee calculation process following the determination of a winner through Chainlink's random word return. The protocol fee in ETH is calculated based on the round's value, entry index, and protocol fee basis points (Bp).

    round.protocolFeeOwed = (round.valuePerEntry * currentEntryIndex * round.protocolFeeBp) / 10_000;

https://github.com/sherlock-audit/2024-01-looksrare/blob/314fd5b753359e07778724342de8dda20713ec2e/contracts-yolo/contracts/YoloV2.sol#L1289

When winners claim their rewards, they can opt to pay the protocol fee in ETH or LOOKS. Paying in LOOKS offers a discount, the rate of which is set by protocolFeeDiscountBp.

    function _protocolFeeOwedInLOOKS(
        uint256 protocolFeeOwedInETH
    ) private view returns (uint256 protocolFeeOwedInLOOKS) {
        protocolFeeOwedInLOOKS =
            (1e18 * protocolFeeOwedInETH * protocolFeeDiscountBp) /
            erc20Oracle.getTWAP(LOOKS, uint32(TWAP_DURATION)) /
            10_000;
    }

https://github.com/sherlock-audit/2024-01-looksrare/blob/314fd5b753359e07778724342de8dda20713ec2e/contracts-yolo/contracts/YoloV2.sol#L1669-L1676

The contract permits the admin to update the protocolFeeDiscountBp, which means the discount rate can change between the time of fee calculation and actual payment by the winner. This adjustment can lead to winners paying a reduced fee if the discount rate is increased after the initial calculation.

    function updateProtocolFeeDiscountBp(uint16 _protocolFeeDiscountBp) external {
        _validateIsOwner();
        _updateProtocolFeeDiscountBp(_protocolFeeDiscountBp);
    }

https://github.com/sherlock-audit/2024-01-looksrare/blob/314fd5b753359e07778724342de8dda20713ec2e/contracts-yolo/contracts/YoloV2.sol#L819-L822

Impact

This scenario creates a potential revenue loss for the protocol and unfairness, as winners from past rounds could benefit from a lower fee due to the retrospective application of increased discount rates.

Code Snippet

https://github.com/sherlock-audit/2024-01-looksrare/blob/314fd5b753359e07778724342de8dda20713ec2e/contracts-yolo/contracts/YoloV2.sol#L1669-L1676

Tool used

Manual review

Recommendation

cache the protocolFeeDiscountBp at the time of initial fee calculation and use this cached value when calculating fees payable in LOOKS.

        function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) internal override {
            ...
            round.winner = round.deposits[currentEntryIndexArray.findUpperBound(winningEntry)].depositor;
            round.protocolFeeOwed = (round.valuePerEntry * currentEntryIndex * round.protocolFeeBp) / 10_000;
+           round.protocolFeeDiscountBp = protocolFeeDiscountBp;
            ...
        }

        function _protocolFeeOwedInLOOKS(
+           Round storage round,
            uint256 protocolFeeOwedInETH
        ) private view returns (uint256 protocolFeeOwedInLOOKS) {
            protocolFeeOwedInLOOKS =
-               (1e18 * protocolFeeOwedInETH * protocolFeeDiscountBp) /
+               (1e18 * protocolFeeOwedInETH * round.protocolFeeDiscountBp) /
                erc20Oracle.getTWAP(LOOKS, uint32(TWAP_DURATION)) /
                10_000;
        }
nevillehuang commented 9 months ago

Invalid, it is admins choice however they want to charge fees. If admin wants to increase fees, then sure, users should be charged more fees for future rounds, seems like intended behavior. Additionally, using LOOKS is a user choice, they can simply use other