code-423n4 / 2023-12-autonolas-findings

3 stars 3 forks source link

Possible DOS when withdrawing liquidity from Solana Lockbox #377

Open c4-bot-2 opened 8 months ago

c4-bot-2 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/liquidity_lockbox.sol#L194-L214

Vulnerability details

Impact

Possible DOS when withdrawing liquidity from Lockbox

Proof of Concept

When withdrawing it is required to pass all the associated accounts in the transaction. But among these (position,pdaPositionAccount and positionMint) are dependent on the current modifiable-state of the account ie. if another withdrawal occurs, the required accounts to be passed to the function call might change resulting in a revert.

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/lockbox-solana/solidity/liquidity_lockbox.sol#L194-L214

    @mutableAccount(pool)
    @account(tokenProgramId)
    @mutableAccount(position)
    @mutableAccount(userBridgedTokenAccount)
    @mutableAccount(pdaBridgedTokenAccount)
    @mutableAccount(userWallet)
    @mutableAccount(bridgedTokenMint)
    @mutableAccount(pdaPositionAccount)
    @mutableAccount(userTokenAccountA)
    @mutableAccount(userTokenAccountB)
    @mutableAccount(tokenVaultA)
    @mutableAccount(tokenVaultB)
    @mutableAccount(tickArrayLower)
    @mutableAccount(tickArrayUpper)
    @mutableAccount(positionMint)
    @signer(sig)
    function withdraw(uint64 amount) external {
        address positionAddress = positionAccounts[firstAvailablePositionAccountIndex];
        if (positionAddress != tx.accounts.position.key) {
            revert("Wrong liquidity token account");
        }

The DOS for a withdrawal can be caused by another user withdrawing before the user's transaction. Due to the possibility to steal fees, attackers would be motivated to frequently call the withdraw method making such a scenario likely.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this it would require a redesign on how the lockbox accepts liquidity. Instead of adding new positions, the lockbox can keep its liquidity in a single position continuously increasing its liquidity for deposits.

Assessed type

Context

c4-pre-sort commented 8 months ago

alex-ppg marked the issue as sufficient quality report

c4-sponsor commented 8 months ago

mariapiamo (sponsor) confirmed

c4-judge commented 7 months ago

dmvt marked the issue as selected for report