code-423n4 / 2024-05-predy-validation

0 stars 0 forks source link

Front-running Risk Leading to Failed Withdrawals Due to Insufficient Collateral #610

Closed c4-bot-10 closed 4 months ago

c4-bot-10 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/ScaledAsset.sol#L67

Vulnerability details

Impact

Users can exploit this by monitoring the mempool and front-running large withdrawals, causing others to face failed transactions.

This creates a race condition where stakers might rush to withdraw their tokens, potentially leading to failed transactions and user frustration.

Proof of Concept


 function removeAsset(AssetStatus storage tokenState, uint256 _supplyTokenAmount, uint256 _amount)
        internal
        returns (uint256 finalBurnAmount, uint256 finalWithdrawAmount)
    {
        if (_amount == 0) {
            return (0, 0);
        }

        require(_supplyTokenAmount > 0, "S3");

        uint256 burnAmount = FixedPointMathLib.mulDivDown(_amount, Constants.ONE, tokenState.assetScaler);

        if (_supplyTokenAmount < burnAmount) {
            finalBurnAmount = _supplyTokenAmount;
        } else {
            finalBurnAmount = burnAmount;
        }

        finalWithdrawAmount = FixedPointMathLib.mulDivDown(finalBurnAmount, tokenState.assetScaler, Constants.ONE);

        require(getAvailableCollateralValue(tokenState) >= finalWithdrawAmount, "S0");

        tokenState.totalCompoundDeposited -= finalBurnAmount;
    }

POC

Initial State:

Staker A's Withdrawal:

Staker A Initiates Withdrawal:

Collateral Check:

The function checks if the available collateral value is sufficient:

require(getAvailableCollateralValue(tokenState) >= finalWithdrawAmount, "S0");

Front-running by Staker B:

Staker B Observes and Acts:

Staker B's Withdrawal Process:

Staker A's Withdrawal Fails:

Staker A's Transaction is Processed:

Collateral Check Fails:

Tools Used

Manual Audit

Recommended Mitigation Steps

Process withdrawals in the order they are received.

Assessed type

MEV

sathishpic22 commented 4 months ago

Hi @alex-ppg

This is well demonstrated with example . Also kind of issues accepted in many contests before. As per my scenario the frontrunning possible.

Please check this thank you

alex-ppg commented 4 months ago

Hey @sathishpic22, thanks for contributing to the PJQA process! This represents a validation repository finding and as such was not evaluated by me directly.

Withdrawals are processed in the order they are received, and it is impossible to avoid the described behavior which is usually known as a "bank run".