Skip to content

Latest commit

 

History

History
68 lines (50 loc) · 2.98 KB

110.md

File metadata and controls

68 lines (50 loc) · 2.98 KB
Raw

Plain Red Puma

Medium

The refund mechanism for exchange fees allows for over refunding due to wrong rounding direction

Summary

The refund mechanism for exchange fees allows for over refunding causing the protocol to receive less fees than intended and the order maker benefitting from that as he gets those funds

Root Cause

return fee - calculateFee(operatorFeeRateBps, outcomeTokens, makerAmount, takerAmount, side);

The subtrahend is rounded down which causes the function to return a bigger refund value than supposed to

Internal pre-conditions

  1. The order fees must be bigger than the minimum - that is expected as the code handles exactly that

External pre-conditions

No external pre-conditions

Attack Path

Not an attack path but the scenario that will occur:

  1. Upon calling acceptLoanOfferAndFillOrder(), we have this code:
            if (exchangeOrder.feeRateBps > minimumOrderFeeRate) {
                uint256 refund = CalculatorHelper.calcRefund(
                    exchangeOrder.feeRateBps,
                    minimumOrderFeeRate,
                    collateralTokenBalanceIncrease,
                    exchangeOrder.makerAmount,
                    exchangeOrder.takerAmount,
                    Side.SELL
                );

                LOAN_TOKEN.safeTransfer(exchangeOrder.maker, refund);
            }
  1. The code wants to refund the difference of fees between what was in the order struct and what is the minimum
  2. We call CalculatorHelper::calcRefund():
        ...
        uint256 fee = calculateFee(orderFeeRateBps, outcomeTokens, makerAmount, takerAmount, side);
        ...
        return fee - calculateFee(operatorFeeRateBps, outcomeTokens, makerAmount, takerAmount, side);
  1. The fee is calculated based on the fee rate in the order struct, this is essentially the fees that were "charged" to the maker or the tokens that are actually in the contract
  2. The subtrahend of the return calculation is the fees based on the minimum fees which are the fees that the protocol wants to actually "charge"
  3. Since the subtrahend rounds down, we can return a value that is bigger than the actual difference we have to return
  4. This causes the protocol to refund more fees than supposed to and the maker will benefit from that as he gets those funds

Impact

Protocol receives less fees than intended and the order maker benefits from that as he gets those funds. Note that while CalculatorHelper is out of scope, according to rules, this is a valid finding:

In case the vulnerability exists in a library and an in-scope contract uses it and is affected by this bug this is a valid issue.

PoC

No response

Mitigation

Round up the subtrahend