Innocent borrower could incur losses caused by a malicious lender

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-04-lavarage/blob/main/libs/smart-contracts/programs/lavarage/src/processor/swap.rs#L12

Vulnerability details

Impact

The protocol allows the lender to change the interest rate anytime. However, since the new interest rate is stored on trading pool level, the lender could front-run a borrowing transaction that's yet to be processed, updating the interest rate too high (up to 99). This is harmful to the borrower even if the borrower repays the SOL immediately.
That's because the minimum elapsed days on repay is set to be one

  let days_elapsed = ((current_timestamp - timestamp) as u64 / 86400) + 1; // +1 to ensure interest is charged from day 0

swapback.rs#L145

Proof of Concept

• Update interest rate on pool level
  lending.rs#L38-L51

• Minimum elapsed days is 1

     let days_elapsed = ((current_timestamp - timestamp) as u64 / 86400) + 1; // +1 to ensure interest is charged from day 0

  swapback.rs#L145

Tools Used

Manual analysis

Recommended Mitigation Steps

Allow the borrower to pass maximum interest rate, this protects the borrower from any change of the interest rate that occur after they send their TX.

Another suggestion: store the interest rate on position level instead.

Assessed type

Other

[c4-sponsor]

piske-alex (sponsor) confirmed

[piske-alex]

Another suggestion: store the interest rate on position level instead.

Will implement max interest rate param. How do I store the interest rate on position before the position is created?

[koolexcrypto]

@c4-sponsor

Thank you for your feedback. That's a very good point, as it can still be front-run.

However, if you still would like to avoid passing the interest rate as a param, interest rate should be stored in trading pool with updated_time, then on borrowing, check if there is not enough timespan between current timestamp and updated_time, revert accordingly. Otherwise, proceed and store the interest rate (for records only).

This should be a sufficient protection without requiring the user to pass max interest rate as a param due to the fact that, a Solana TX has an expiration time. So, if it is not processed within a certain time, it will never be.

During transaction processing, Solana Validators will check if each transaction's recent blockhash is recorded within the most recent 151 stored hashes (aka "max processing age"). If the transaction's recent blockhash is older than this max processing age, the transaction is not processed.

Check this for more info on this:
https://solana.com/docs/advanced/confirmation#how-does-transaction-expiration-work

[c4-judge]

alcueca marked the issue as satisfactory

[c4-judge]

alcueca marked the issue as selected for report

[Arabadzhiew]

I don't think that front-running is currently possible on Solana. That is because the chain does not have a public mempool and the transaciton fees on it are fixed (i.e. validators can not be tipped in order to be bribed). There was the possibility to use the Jito bundler in order to achieve front-running up until recently, as it had a public mempool that could be used for that, but as of March this year they actually decided to suspend that service.

Therefore, I believe that this issue is actually invalid.

CC: @alcueca @koolexcrypto

[koolexcrypto]

Hi @Arabadzhiew

The possibility is still there, you can't relay on external params for safety. Even if you guarantee that there is no public mempool other than Jito, validators can still front-run users.

Regarding

the transaciton fees on it are fixed (i.e. validators can not be tipped in order to be bribed )

That was in the past,Please check priority fees in Solana

A Solana transaction can include an optional fee to prioritize itself against others known as a "prioritization fee". Paying this additional fee helps boost how a transaction is prioritized against others, resulting in faster execution times

https://solana.com/docs/core/fees#prioritization-fee

[alcueca]

Front-running by validators is possible in Solana, and after a brief analysis of the current stituation, concerning to some users. This issue can cause mild losses to users. Nothing major, but a headache for the protocol that will have to deal with the complaints and possibly refunds. Affected users would have to close their positions immediately if they notice the issue. All in all, a medium is a fair severity rating.