search

code-423n4 / 2024-04-lavarage-findings

2 stars 2 forks source link

Innocent borrower could incur losses caused by a malicious lender #17

Open c4-bot-10 opened 4 months ago

c4-bot-10 commented 4 months ago

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

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 commented 4 months ago

piske-alex (sponsor) confirmed

piske-alex commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

alcueca marked the issue as satisfactory

c4-judge commented 4 months ago

alcueca marked the issue as selected for report

Arabadzhiew commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

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.