code-423n4 / 2023-01-ondo-findings

0 stars 0 forks source link

`repayBorrowBehalf()` can be front-run, resulting in `caller` paying more than expected #358

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/tokens/cToken/CTokenModified.sol#L751

Vulnerability details

Impact

If a payer calls repayBorrowBehalf() with amount == uint(-1), the call can be frontRun by the borrower with a borrow() call, increasing accountBorrows[borrower].principal, leading to the payer paying more than expected.

That is because here, the amount the payer will pay is set to the borrower's balance (when amount = uint(-1)).

The payer paid more than expected, meaning the borrower essentially stole the additional borrow balance he created with the front run borrow() call.

Because of KYC, this issue is less likely to happen, but it is still profitable, and the malicious user can back run both calls with a redeem() call in fToken to retrieve all their underlying before being added to a sanctionsList.

Note that it is contingent on repayer approving fToken a high enough amount for the attack to happen, but because the amount to repay() is unknown at the time of function call (because of interests not accrued yet), it is likely the payer will approve a higher value anyway.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding some sort of "slippage" in repayBorrowBehalf() , ie the maximum amount the payer agrees to repay for the borrower.

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2023-01-ondo-findings/issues/341

c4-judge commented 1 year ago

trust1995 marked the issue as grade-b

trust1995 commented 1 year ago

By submitting the call with -1, payer yields any slippage protection the system offers.

c4-judge commented 1 year ago

trust1995 marked the issue as grade-a