search

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

5 stars 2 forks source link

Swap in router uses pay_amt to swap but allows msg.value to be larger #401

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/6a0e1d89ff710f81428228d132e7d0c42de3a3cd/contracts/RubiconRouter.sol#L505

Vulnerability details

Impact

Users can lose funds

Proof of Concept

In the swapWithEth() function there is a pay amount input value is used to make the swap. msg.value is required to be greater than or equal to the pay amount plus the fee. If msg. value is greater then the extra ETH will be lost.

https://github.com/RubiconDeFi/rubicon-protocol-v1/blob/6a0e1d89ff710f81428228d132e7d0c42de3a3cd/contracts/RubiconRouter.sol#L505

Recommended Mitigation Steps

require msg.value == amtWithFee instead of >= or directly set pay amount equal to msg.value - fee

KenzoAgada commented 2 years ago

Duplicate of #15.

bghughes commented 2 years ago

Duplicate of #15