Closed code423n4 closed 1 year ago
We can see that Compound doesn't do it either: https://github.com/compound-finance/compound-protocol/blob/a3214f67b73310d547e00fc578e8355911c9d376/contracts/CToken.sol Will treat as more of a suggestion.
trust1995 marked the issue as grade-b
trust1995 marked the issue as grade-a
Lines of code
https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/tokens/cToken/CTokenModified.sol#L254 https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/tokens/cToken/CTokenModified.sol#L267
Vulnerability details
Impact
borrowRatePerBlock()
may return an incorrect value, astotalBorrows
may not be up to date : that is, ifaccrueInterest()
was called N >= 1 blocks before the call toborrowRatePerBlock()
.Rated as medium as external dApps may call
borrowRatePerBlock()
and have built-in logic to borrow depending on the value of the returned interest rate.Proof of Concept
DAI
to protocol XYZ, where the lending interest rate isN
DAI
fromFlux
, but only if the borrowing interest rate is less than the lending interest rate on XYZ (to make it profitable)borrowRatePerBlock
to check whether the current per-block borrowing interest rate is less than N (to see whether it is profitable to borrow fromFlux
).borrowRatePerBlock
returns M < N, so the protocol call goes through by callingborrow()
, then lends this amount toXYZ
.L > N
, making this whole operation non-profitable for ABC.This would not have happened if the interest had been updated in
borrowRatePerBlock
.Tools Used
Manual review
Recommended Mitigation Steps
Add a
accrueInterest()
call at the start ofborrowRatePerBlock()
andsupplyRatePerBlock()