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

2 stars 2 forks source link

Oracle is payer of liquidation transaction fee #12

Open c4-bot-9 opened 4 months ago

c4-bot-9 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#L113

Vulnerability details

Proof of Concept

For Liquidate context oracle address is set as Signer. This means that this account will pay for the transaction. But this should not be like that, as protocol doesn't charge fees(only UI fees that can be omitted and any other UI can be built and used).

Using oracle API users will be able to fetch transactions signed by oracle and then execute them. All of this txs fees should be paid by oracle. After discussing with sponsor i have found out that oracle account should not hold funds, which means that liquidations won't work.

Instead, traders should sign tx as well and pay for tx.

Impact

Oracle pays for tx, but should not and don't have funds.

Tools Used

VsCode

Recommended Mitigation Steps

Make trader pay the fee.

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

Hi @rvierdiiev

It is not possible to let the trader pay the fee unless the transaction is signed by the trader. Why would the trader sign a transaction that will lead to liquidate their position? This will lead simply to DoS liquidation.

I don't see this an issue here. Oracle which is the only signer should hold funds in order to be able to send TXs to the chain. Or additional account (controlled by the protocol only) which holds funds to pay for the fee. Which takes us back to the same point that, a controlled account by the protocol should hold the funds (Oracle or additional signer).

rvierdiiev commented 4 months ago

When i said trader, i meant owner of trading pool and not position holder. And Oracle is not going to hold funds, this is stated by protocol team.

alcueca commented 4 months ago

liquidations won't work

Could I get a PoC that if the oracle doesn't hold funds then the liquidations don't work?

Otherwise, I'll mark this as invalid.

rvierdiiev commented 4 months ago

Sure @alcueca

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

Transactions are required to have at least one account which has signed the transaction and is writable. Writable signer accounts are serialized first in the list of transaction accounts and the first of these accounts is always used as the "fee payer".

In anchor, transaction signer is fetched by Signer struct. This struct checks, that account has signed transaction(note that many accounts can sign same tx on Solana). In case of liquidate function, which receives Liquidate context as input, we can see that the only Signer is oracle account and no one else. This means that oracle account's balance will be reduced with transaction fees and if oracle doesn't have funds, then tx will fail.

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

Before any transaction instructions are processed, the fee payer account balance will be deducted to pay for transaction fees. If the fee payer balance is not sufficient to cover transaction fees, the transaction will be dropped by the cluster.

alcueca commented 4 months ago

Gotcha, then I assume that if we want the transaction to still be signed by the oracle, so that the liquidation price can be trusted, then liquidate should receive more than one signer account. The first one should be the liquidator, paying for fees, and the second one the oracle, to ensure trusted liquidation data.

alcueca commented 3 months ago

The net effect of this issue is that Lavarage would need to provide an small amount of funds. Given that the average price per transaction in Solana is way below $1, this is a dust amount and negligible for the protocol. Users are not impacted. The result is no more than a deviation from expected design.

c4-judge commented 3 months ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

alcueca marked the issue as grade-a

c4-judge commented 3 months ago

alcueca marked the issue as not selected for report

rvierdiiev commented 3 months ago

@alcueca as i already stated, protocol was not going to hold any funds on Oracle account and was not aware of the fact that Oracle will be fee payer

as result first liquidations would not work, before they figure this out and top up Oracle account because of that users were impacted as positions could not be liquidated on time.