Tapioca DAO - iglyx's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/curve/TricryptoNativeStrategy.sol#L151-L179 https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/curve/TricryptoLPStrategy.sol#L160-L196

Vulnerability details

Summary

compound() has no access controls and performs swapping with the strategy reward funds. This can be gamed by an attacker as the pool can be atomically manipulated to provide significantly off-market price for the swap.

Vulnerability Detail

An attacker can skew the pool and call compound(), which will execute the rewardToken to wrappedNative swap at an unfavourable price, benefitting the attacker, who will return the pool to initial state after the call, pocketing the crvAmount * price_difference of value. This way a self-called sandwiching is possible: attacker will run compound() whenever there is a big enough amount of rewards due and sandwich this transaction.

Also, minAmount is calculated off the pool data and provides no protection against the attack.

Impact

Strategy CRV rewards due can be stolen this way.

Code Snippet

TricryptoNativeStrategy's compound() has no access controls and swaps minted CRV on the market:

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/curve/TricryptoNativeStrategy.sol#L151-L179

@>  function compound(bytes memory) public {
        uint256 claimable = lpGauge.claimable_tokens(address(this));
        if (claimable > 0) {
            uint256 crvBalanceBefore = rewardToken.balanceOf(address(this));
            minter.mint(address(lpGauge));
            uint256 crvBalanceAfter = rewardToken.balanceOf(address(this));

            if (crvBalanceAfter > crvBalanceBefore) {
                uint256 crvAmount = crvBalanceAfter - crvBalanceBefore;

                ISwapper.SwapData memory swapData = swapper.buildSwapData(
                    address(rewardToken),
                    address(wrappedNative),
                    crvAmount,
                    0,
                    false,
                    false
                );
                uint256 calcAmount = swapper.getOutputAmount(swapData, "");
@>              uint256 minAmount = calcAmount - (calcAmount * 50) / 10_000; //0.5%
@>              swapper.swap(swapData, minAmount, address(this), "");

                uint256 queued = wrappedNative.balanceOf(address(this));

                _addLiquidityAndStake(queued);
                emit AmountDeposited(queued);
            }
        }
    }

The same is for TricryptoLPStrategy:

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/curve/TricryptoLPStrategy.sol#L160-L196

@>  function compound(bytes memory) public {
        uint256 claimable = lpGauge.claimable_tokens(address(this));
        if (claimable > 0) {
            uint256 crvBalanceBefore = rewardToken.balanceOf(address(this));
            minter.mint(address(lpGauge));
            uint256 crvBalanceAfter = rewardToken.balanceOf(address(this));

            if (crvBalanceAfter > crvBalanceBefore) {
                uint256 crvAmount = crvBalanceAfter - crvBalanceBefore;

                ISwapper.SwapData memory swapData = swapper.buildSwapData(
                    address(rewardToken),
                    address(wrappedNative),
                    crvAmount,
                    0,
                    false,
                    false
                );
@>              uint256 calcAmount = swapper.getOutputAmount(swapData, "");
                uint256 minAmount = calcAmount - (calcAmount * 50) / 10_000; //0.5%
                swapper.swap(swapData, minAmount, address(this), "");

                uint256 wrappedNativeAmount = wrappedNative.balanceOf(
                    address(this)
                );
                calcAmount = lpGetter.calcWethToLp(wrappedNativeAmount);
@>              minAmount = calcAmount - (calcAmount * 50) / 10_000; //0.5%
                uint256 lpAmount = lpGetter.addLiquidityWeth(
                    wrappedNativeAmount,
                    minAmount
                );
                lpGauge.deposit(lpAmount, address(this), false);

                emit AmountDeposited(lpAmount);
            }
        }
    }

Minimum amount calculated from the pool data, which can be skewed by the attacker, so such approach provides no real control:

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/68f20cbbf24ac42d2a54660c70b0a74301bee55b/contracts/Swapper/CurveSwapper.sol#L56-L76

    /// @notice Computes amount out for amount in
    /// @param swapData operation data
    /// @param dexOptions AMM data
    function getOutputAmount(
        SwapData calldata swapData,
        bytes calldata dexOptions
    ) external view override returns (uint256 amountOut) {
        uint256[] memory tokenIndexes = abi.decode(dexOptions, (uint256[]));

        (uint256 amountIn, ) = _getAmounts(
            swapData.amountData,
            swapData.tokensData.tokenInId,
            swapData.tokensData.tokenOutId,
            yieldBox
        );
>>      amountOut = curvePool.get_dy(
            int128(int256(tokenIndexes[0])),
            int128(int256(tokenIndexes[1])),
            amountIn
        );
    }

Tool used

Manual Review

Recommendation

Consider allowing for permissioned runs of compound() only, and basing minAmount requirement on the input from this trusted caller.

Additionally comparing the provided minimum with Oracle price is advised to deal with semi-trusted callers (this will not eliminate the surface, but will limit the damage).

Assessed type

Access Control