search

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

2 stars 2 forks source link

Liquidation doesn't have expiration #6

Open c4-bot-8 opened 4 months ago

c4-bot-8 commented 4 months ago

Lines of code

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

Vulnerability details

Proof of Concept

When liquidation happens, then user should query oracle API and provide address of account that should be liquidated. Then oracle signs transaction and anyone can execute it later.

There is a check inside liquidation function that LTV is more than 90%. This is the condition to execute liquidation. When oracle signs transaction it also provides position_size, which is current amount of sol that position holds.

As you know prices can change quickly, which means that position_size can be not same in 10 minutes. But the problem is in case if transaction was signed when position was unhealthy, but now used and after that position became healthy, then still anyone can execute old signed transaction with not up to date position_size and as result account will be liquidated and borrower will loose funds.

Impact

Position can be liquidated, when it became healthy again.

Tools Used

VsCode

Recommended Mitigation Steps

I believe that oracle signatures should work for some short time period, so users need to query new one to get up to date info about account.

Assessed type

Error

c4-sponsor commented 4 months ago

piske-alex (sponsor) confirmed

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

koolexcrypto commented 4 months ago

I believe this is invalid 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. So old signed transaction will be ignored by validators. This is different from EVM where the TX can still be valid in future if no time check is done on smart contract level.

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

rvierdiiev commented 4 months ago

thank you for teaching me Solana internals, i agree with you.

koolexcrypto commented 4 months ago

I appreciate it @rvierdiiev , Thank you for your patience and understanding.

piske-alex commented 4 months ago

I agree @koolexcrypto seems this doesn't need a fix

alcueca commented 4 months ago

Even if transactions expire, the can be liquidated on stale data within the expiration time. That would be quite unfair (unless stated by the sponsor).

koolexcrypto commented 4 months ago

@alcueca

Some points to consider here:

cc: @piske-alex

alcueca commented 4 months ago

@koolexcrypto, it is a very strange design where the health of the position is not checked during liquidation execution, but at a previous time (however short). Even if we are talking about 60 to 90 seconds, it is possible that positions will be liquidated when they were unhealthy, but not anymore.

However, I do realize that that could happen only in very rare cases, so I'll downgrade this to QA, hoping that a more robust design can be found.

c4-judge commented 4 months ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 4 months ago

alcueca marked the issue as grade-a

koolexcrypto commented 4 months ago

@alcueca

Totally agree with what you have stated. Thank you for allowing this conversation to happen.

c4-judge commented 4 months ago

alcueca marked the issue as not selected for report