Tapioca DAO - jasonxiale's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Swapper/CurveSwapper.sol#L94-L142

Vulnerability details

Impact

AMMs should provide their users with an option to limit the execution of their pending actions, such as swaps or adding and removing liquidity. The most common solution is to include a deadline timestamp as a parameter (for example see Uniswap V2). If such an option is not present, users can unknowingly perform bad trades

Proof of Concept

The function CurveSwapper.swap doesn't set any deadline related parameter

    function swap(
        SwapData calldata swapData,
        uint256 amountOutMin,
        address to,
        bytes memory data
    ) external override returns (uint256 amountOut, uint256 shareOut) {
        // Get Curve tokens' indexes & addresses
        uint256[] memory tokenIndexes = abi.decode(data, (uint256[]));
        address tokenIn = curvePool.coins(tokenIndexes[0]);
        address tokenOut = curvePool.coins(tokenIndexes[1]);

        // Get tokens' amounts
        (uint256 amountIn, ) = _getAmounts(
            swapData.amountData,
            swapData.tokensData.tokenInId,
            swapData.tokensData.tokenOutId,
            yieldBox
        );

        // Retrieve tokens from sender or from YieldBox
        amountIn = _extractTokens(
            swapData.yieldBoxData,
            yieldBox,
            tokenIn,
            swapData.tokensData.tokenInId,
            amountIn,
            swapData.amountData.shareIn
        );

        // Swap & compute output
        amountOut = _swapTokensForTokens(
            int128(int256(tokenIndexes[0])),
            int128(int256(tokenIndexes[1])),
            amountIn,
            amountOutMin
        );
        if (swapData.yieldBoxData.depositToYb) {
            _safeApprove(tokenOut, address(yieldBox), amountOut);
            (, shareOut) = yieldBox.depositAsset(
                swapData.tokensData.tokenOutId,
                address(this),
                to,
                amountOut,
                0
            );
        } else {
            IERC20(tokenOut).safeTransfer(to, amountOut);
        }
    }

Tools Used

VS

Recommended Mitigation Steps

add deadline related parameter

Assessed type

Timing

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/modules/MagnetarMarketModule.sol#L677-L732

Vulnerability details

Impact

token mights stuck in MagnetarMarketModule contract if the asset doesn't support cross-chain operation

Proof of Concept

[MagnetarMarketModule._withdrawToChain] will check if the asset supports a cross chain operation in L703-L709, if the asset doesn't support, the function will return.

Taking one of the _withdrawToChain callers MagnetarMarketModule.depositRepayAndRemoveCollateralFromMarket as an example: While MagnetarMarketModule.depositRepayAndRemoveCollateralFromMarket is called, the function will call MagnetarMarketModule._depositRepayAndRemoveCollateralFromMarket, supposed everything goes well, the control flow will fall into L258-L289. Suppose collateralAmount in L258 is not zero, and withdrawCollateralParams.withdraw is true, in such case, collateralWithdrawReceiver will be address(this). After calling marketInterface.removeCollateral, address(this) which is MagnetarMarketModule contract will own the asset. Since withdrawCollateralParams.withdraw is true, MagnetarMarketModule._withdrawToChain will be called.

212     function _depositRepayAndRemoveCollateralFromMarket(
213         address market,
214         address user,
215         uint256 depositAmount,
216         uint256 repayAmount,
217         uint256 collateralAmount,
218         bool extractFromSender,
219         ICommonData.IWithdrawParams calldata withdrawCollateralParams
220     ) private {

            ...

256         // performs a removeCollateral operation on the market
257         // if `withdrawCollateralParams.withdraw` it uses `withdrawTo` to withdraw collateral on the same chain or to another one
258         if (collateralAmount > 0) {
259             address collateralWithdrawReceiver = withdrawCollateralParams
260                 .withdraw
261                 ? address(this)
262                 : user;
263             uint256 collateralShare = yieldBox.toShare(
264                 marketInterface.collateralId(),
265                 collateralAmount,
266                 false
267             );
268             marketInterface.removeCollateral(
269                 user,
270                 collateralWithdrawReceiver,
271                 collateralShare
272             );
273 
274             //withdraw
275             if (withdrawCollateralParams.withdraw) {
276                 _withdrawToChain(
277                     yieldBox,
278                     collateralWithdrawReceiver,
279                     marketInterface.collateralId(),
280                     withdrawCollateralParams.withdrawLzChainId,
281                     LzLib.addressToBytes32(user),
282                     collateralAmount,
283                     collateralShare,
284                     withdrawCollateralParams.withdrawAdapterParams,
285                     payable(this),
286                     address(this).balance
287                 );
288             }
289         }
290     }

In L703-L709, the function will return which leaves the asset in the MagnetarMarketModule contract

677     function _withdrawToChain(
678         IYieldBoxBase yieldBox,
679         address from,
680         uint256 assetId,
681         uint16 dstChainId,
682         bytes32 receiver,
683         uint256 amount,
684         uint256 share,
685         bytes memory adapterParams,
686         address payable refundAddress,
687         uint256 gas
688     ) private {
        ...
702         // make sure the asset supports a cross chain operation
703         try
704             IERC165(address(asset)).supportsInterface(
705                 type(ISendFrom).interfaceId
706             )
707         {} catch {
708             return;        <---------------- here
709         }
710
    ...

Tools Used

VS

Recommended Mitigation Steps

If the asset doesn't support cross-chain, maybe we can perform a withdraw as L690-L699

--- MagnetarMarketModule.sol	2023-08-04 17:22:52.272395098 +0800
+++ MagnetarMarketModule_new.sol	2023-08-04 17:34:57.230800699 +0800
@@ -705,6 +705,13 @@
                 type(ISendFrom).interfaceId
             )
         {} catch {
+            yieldBox.withdraw(
+                assetId,
+                from,
+                LzLib.bytes32ToAddress(receiver),
+                amount,
+                share
+            );
             return;
         }

Assessed type

Other

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/compound/CompoundStrategy.sol#L115 https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/compound/CompoundStrategy.sol#L145-L145

Vulnerability details

Impact

Contract CompoundStrategy uses exchangeRateStored to determinate the exchange range in here and here. But according to cToken's API document, This function does not accrue interest before calculating the exchange rate, so function CompoundStrategy._currentBalance and CompoundStrategy._withdraw use stale value.

Proof of Concept

CompoundStrategy._currentBalance:

    function _currentBalance() internal view override returns (uint256 amount) {
        uint256 shares = cToken.balanceOf(address(this));
        uint256 pricePerShare = cToken.exchangeRateStored(); <--------------- Here
        uint256 invested = (shares * pricePerShare) / (10 ** 18);
        uint256 queued = wrappedNative.balanceOf(address(this));
        return queued + invested;
    }

CompoundStrategy._withdraw:

    function _withdraw(
        address to,
        uint256 amount
    ) internal override nonReentrant {
        uint256 available = _currentBalance();
        require(available >= amount, "CompoundStrategy: amount not valid");

        uint256 queued = wrappedNative.balanceOf(address(this));
        if (amount > queued) {
            uint256 pricePerShare = cToken.exchangeRateStored(); <--------------- Here
            uint256 toWithdraw = (((amount - queued) * (10 ** 18)) /
                pricePerShare);

            cToken.redeem(toWithdraw);
            INative(address(wrappedNative)).deposit{
                value: address(this).balance
            }();
        }

        require(
            wrappedNative.balanceOf(address(this)) >= amount,
            "CompoundStrategy: not enough"
        );
        wrappedNative.safeTransfer(to, amount);

        emit AmountWithdrawn(to, amount);
    }

Tools Used

VS

Recommended Mitigation Steps

You shoud use exchangeRateCurrent() instead

Assessed type

Math

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/tokens/TapOFT.sol#L225

Vulnerability details

Impact

TapOFT.extractTAP uses wrong check to determine the number of tokens it can mint for specified week, which causes it mints more tokens than it's supposed.

Proof of Concept

According to comments in line 55 and the source code, emissionForWeek should be the amount of emitted TAP for a specific week However while emissionForWeek is used in function TapOFT.extractTAP, is misused as the limit of one tx

    function extractTAP(address _to, uint256 _amount) external notPaused {
        require(msg.sender == minter, "unauthorized");
        require(_amount > 0, "amount not valid");

        uint256 week = _timestampToWeek(block.timestamp);
        require(emissionForWeek[week] >= _amount, "exceeds allowable amount"); <-------------- Here is the wrong check
        _mint(_to, _amount);
        mintedInWeek[week] += _amount;
        emit Minted(msg.sender, _to, _amount);
    }

Tools Used

VS

Recommended Mitigation Steps

--- TapOFT.sol	2023-08-02 22:52:48.149365288 +0800
+++ TapOFT_New.sol	2023-08-02 22:53:29.974237715 +0800
@@ -222,7 +222,7 @@
         require(_amount > 0, "amount not valid");
 
         uint256 week = _timestampToWeek(block.timestamp);
-        require(emissionForWeek[week] >= _amount, "exceeds allowable amount");
+        require(emissionForWeek[week] >= _amount + mintedInWeek[week], "exceeds allowable amount");
         _mint(_to, _amount);
         mintedInWeek[week] += _amount;
         emit Minted(msg.sender, _to, _amount);

Assessed type

Invalid Validation

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/option-airdrop/AirdropBroker.sol#L535 https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/TapiocaOptionBroker.sol#L555

Vulnerability details

Impact

There are some weird token that has decimals larger than 18(e.g. YAM-V2 has 24), if such token is used in the protocol, the function will revert.

Proof of Concept

Take TapiocaOptionBroker._getDiscountedPaymentAmount as an example: In L555, the paymentAmount is calculated as paymentAmount = paymentAmount / (10 ** (18 - _paymentTokenDecimals));, if _paymentTokenDecimals is larger than 18, the function will revert.

    function _getDiscountedPaymentAmount(
        uint256 _otcAmountInUSD,
        uint256 _paymentTokenValuation,
        uint256 _discount,
        uint256 _paymentTokenDecimals
    ) internal pure returns (uint256 paymentAmount) {
        // Calculate payment amount
        uint256 rawPaymentAmount = _otcAmountInUSD / _paymentTokenValuation;
        paymentAmount =
            rawPaymentAmount -
            muldiv(rawPaymentAmount, _discount, 100e4); // 1e4 is discount decimals, 100 is discount percentage
        paymentAmount = paymentAmount / (10 ** (18 - _paymentTokenDecimals)); <-------------- Here
    }

Tools Used

VS

Recommended Mitigation Steps

Assessed type

Under/Overflow