code-423n4 / 2022-05-rubicon-findings

5 stars 2 forks source link

Calculation error #445

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/master/contracts/RubiconRouter.sol#L282

Vulnerability details

Impact

    function _swap(
        uint256 pay_amt,
        uint256 buy_amt_min,

maxAmount is a pay_amt denominated in token route[0] (=pay_gem)

buy_amt_min is a buy_amt denominated in token route[last] (=buy_gem)

buy_amt_min * expectedMarketFeeBPS / 1000 
is the fee denominated in token `route[last]`(=buy_gem)
-- takers pay fees with the token they receive from the swap

Line 282 subtracts a buy_gem amount (the fee) from a pay_gem amount, which makes no sense. Moreover, the fee is taken in pay_gems (from the tokens sent by the taker) so it has no place being mentioned in a buy_amt_min argument.

Recommended Mitigation Steps

Set buy_amt_min argument of _swap to buy_amt_min that was given as argument to this function (swapEntireBalance).

bghughes commented 2 years ago

Duplicate of #248

HickupHH3 commented 2 years ago

duplicate of #52