code-423n4 / 2023-07-tapioca-findings

13 stars 9 forks source link

Onchain slippage calculation is same as Zero slippage and doest not protect against frontrunning #1219

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/aave/AaveStrategy.sol#L193 https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/convex/ConvexTricryptoStrategy.sol#L207 https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/curve/TricryptoNativeStrategy.sol#L171

Vulnerability details

Impact

The strategy uses the current pool value to calculate it amount out which can always be manipulated by flashloans .Such swaps can be frontrunned making the strategy excute at bad price.

Proof of Concept

  function getOutputAmount(
        SwapData calldata swapData,
        bytes calldata
    ) external view override returns (uint256 amountOut) {
        (address tokenIn, address tokenOut) = _getTokens(
            swapData.tokensData,
            yieldBox
        );
        address[] memory path = _createPath(tokenIn, tokenOut);
        (uint256 amountIn, ) = _getAmounts(
            swapData.amountData,
            swapData.tokensData.tokenInId,
            swapData.tokensData.tokenOutId,
            yieldBox
        );

        uint256[] memory amounts = swapRouter.getAmountsOut(amountIn, path);
        amountOut = amounts[1];
    }

This only uses the current pool value to get amount out and can be frontRunned to make bad trades

Tools Used

manuel review

Recommended Mitigation Steps

Use trusted oracles or make miniAmount an input for it.

Assessed type

Uniswap

c4-pre-sort commented 1 year ago

minhquanym marked the issue as duplicate of #158

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Insufficient quality

c4-judge commented 1 year ago

dmvt changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

dmvt marked the issue as satisfactory

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Insufficient quality

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #245