code-423n4 / 2023-11-shellprotocol-findings

7 stars 7 forks source link

New OceanAdapter and Curve2PoolAdapter may enable front-running attacks, stealing user funds and manipulating swap rates. #89

Closed c4-bot-10 closed 11 months ago

c4-bot-10 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/OceanAdapter.sol#L55-L76

Vulnerability details

Impact

New OceanAdapter interface and sample Curve adapters could allow attackers to steal funds from users and manipulate expected swap rates.

Let's see this example scenario as an example.

contract Curve2PoolAdapter is OceanAdapter {

    function computeOutputAmount(
        uint256 inputToken,
        uint256 outputToken,
        uint256 inputAmount,
        bytes32 minimumOutputAmount
    ) internal override returns (uint256 outputAmount){

        uint256 indexInput = indexOf[inputToken];
        uint256 indexOutput = indexOf[outputToken];

        outputAmount = curvePool.swap(
            indexInput, 
            indexOutput,
            inputAmount,
            minimumOutputAmount
        );
    }

}

contract MaliciousAdapter is OceanAdapter {

    function computeOutputAmount(
        uint256 inputToken,
        uint256 outputToken,
        uint256 inputAmount,
        bytes32 minimumOutputAmount
    ) internal override returns (uint256 outputAmount) {

        // Manipulate amount sent to pool 
        uint256 maliciousInputAmount = inputAmount * 2;

        outputAmount = curvePool.swap(
            indexInput,
            indexOutput,
            maliciousInputAmount,
            minimumOutputAmount
        );

        // Keep excess tokens
        skimOffTheTop(inputAmount);

    }

}

The MaliciousAdapter can manipulate input amounts sent to external protocols like Curve and front-run user transactions for profit.

Proof of Concept

The OceanAdapter.sol computeOutputAmount() method used for primitive integrations.

OceanAdapter.sol#computeOutputAmount

    function computeOutputAmount(
        uint256 inputToken,
        uint256 outputToken,
        uint256 inputAmount,
        address,
        bytes32 metadata
    )
        external
        override
        onlyOcean
        returns (uint256 outputAmount)
    {
        unwrapToken(inputToken, inputAmount);

        // handle the unwrap fee scenario
        uint256 unwrapFee = inputAmount / IOceanInteractions(ocean).unwrapFeeDivisor();
        uint256 unwrappedAmount = inputAmount - unwrapFee;

        outputAmount = primitiveOutputAmount(inputToken, outputToken, unwrappedAmount, metadata);

        wrapToken(outputToken, outputAmount);
    }

Expected Behavior

Accept user's input amount Interact with external protocol using user's input amount Provide output amount to user based on protocol rates

  // No validation of inputAmount 
  externalProtocol.swap(inputAmount);

}

This allows the MaliciousAdapter to front-run and profit.

  // Doubles user's input amount
  uint256 maliciousInput = inputAmount * 2;

  externalProtocol.swap(maliciousInput);

  // Keep difference for themselves
  skimOffTheTop(inputAmount); 

}

Observed Behavior

Attacker manipulates input amount sent to external protocol Attacker profits from spread between user's amount and manipulated amount User receives improperly priced output

This enables frontrunning and manipulation of expected output.

Tools Used

Vs

Recommended Mitigation Steps

Assessed type

Other

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #78

c4-judge commented 11 months ago

0xA5DF marked the issue as unsatisfactory: Invalid