Tapioca DAO - rvierdiiev's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L282-L290

Vulnerability details

Impact

BigBang.addCollateral allows to add any amount on behalf of owner without allowance. Any user can steal funds.

Proof of Concept

BigBang.addCollateral function can be used to add collateral on behalf of owner. https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L282-L290

    function addCollateral(
        address from,
        address to,
        bool skim,
        uint256 amount,
        uint256 share
    ) public allowedBorrow(from, share) notPaused {
        _addCollateral(from, to, skim, amount, share);
    }

There are share and amount params that caller can provide. In case if share param is 0, then amount is converted to shares.

addCollateral function can be called on behalf of owner. That's why allowedBorrow(from, share) modifier is called to check allowance. The problem is that caller can provide 0 as share and as result he will always pass allowance. Because of that he can spend any amount of from that is approved to BigBang. For example, he can add this collateral to himself using to param.

Tools Used

VsCode

Recommended Mitigation Steps

Check approval when you have calculated shares.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L118-L125

Vulnerability details

Impact

LidoEthStrategy._currentBalance can be manipulated by changing price in curve pool. This allows attacker to withdraw more funds.

Proof of Concept

LidoEthStrategy._currentBalance is used to know how many eth is controlled by startegy. https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L118-L125

    function _currentBalance() internal view override returns (uint256 amount) {
        uint256 stEthBalance = stEth.balanceOf(address(this));
        uint256 calcEth = stEthBalance > 0
            ? curveStEthPool.get_dy(1, 0, stEthBalance)
            : 0;
        uint256 queued = wrappedNative.balanceOf(address(this));
        return calcEth + queued;
    }

So when stEthBalance is found, then function converts it to eth curve pool.

This is incorrect, because attacker can manipulate price inside the pool using flashloan and then use that in order to mint more shares or withdraw more shares from strategy. In both cases it will loss of funds for other stakers.

Tools Used

VsCode

Recommended Mitigation Steps

Don't use pool price for conversion.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L560-L637

Vulnerability details

Impact

BigBang._liquidateUser doesn't burn liquidated USDO and creates insolvency.

Proof of Concept

When user is going to be liquidated, then his debt that can be liquidated in USDO and collateral amount that can be spent is calculated.

This actually means, that collateralShare amount should be traded in order to cover borrowPart of USDO that is held by owner. When swap is done then swapped USDO is received by BigBang. Then liquidator and protocol receive their fees from extra shares.

Nothing else is done in the _liquidateUser function. The problem is that total borrow amount of USDO is decreased, but liquidated part was not burnt. As result, this amount is still present in total circulation of USDO, but it's not backed by collateral anymore. And also as this amount is inside contract it is assumed to be protocol fee, which is wrong.

Tools Used

VsCode

Recommended Mitigation Steps

Burn liquidated USDO amount.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L309-L326 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L605

Vulnerability details

Impact

BigBang.liquidate and SGLLiquidation.liquidate have only 1 swap data param for different swaps. As swap data is used to retrieve min amount after swap, liquidator can't provide slippage for all trades.

Proof of Concept

Liquidator can liquidate several users in same time using liquidate function. Pls, note that liquidator provides one swapper and 1 collateralToAssetSwapData. But he provides arrays for users and their debts.

Later, this collateralToAssetSwapData will be used to fetch minAssetMount that liquidator wants to receive from the swap. Because, collateralToAssetSwapData is not array, that means that user can provide only same min amount for all trades, which is not correct. Because of that liquidators will have to use big min amout, to not revert all calls or do liquidations only for 1 user a time.

Tools Used

VsCode

Recommended Mitigation Steps

Allow liquidator to provide array of min amounts.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/aave/AaveStrategy.sol#L129-L132

Vulnerability details

Impact

AaveStrategy does not provide approval for new swapper.

Proof of Concept

When new swapper is provided to AaveStrategy then approval is not set for it.

This is needed and it's done for original swapper in constructor.

Because of that swapper will not work and withdrawing will not work anymore, because it will be not possible to claim rewards.

Tools Used

VsCode

Recommended Mitigation Steps

Provide approve for new swapper and remove approve for old one.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/MagnetarV2.sol#L236-L238

Vulnerability details

Impact

In case of native token wrapping to tOFT msg.value is incremented. Because of that tx will revert.

Proof of Concept

MagneatrV2.burst allows to call several actions in a row. Function increments valAccumulator for each call. This is needed in order to check, that user provided enough funds for the call.

The problem is that when user wants to wrap native asset into tOFT, then valAccumulator is increased second time for the call.

Because of that check in the end will fail.

Tools Used

VsCode

Recommended Mitigation Steps

Do not need to add value again.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Swapper/UniswapV2Swapper.sol#L47-L54

Vulnerability details

Impact

UniswapV2Swapper and UniswapV3Swapper getDefaultDexOptions function can't handle deadline. Swap can be sandwiched.

Proof of Concept

UniswapV2Swapper.swap function has data param. This param can contain deadline. If it's empty the deadline will be generated by getDefaultDexOptions function.

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Swapper/UniswapV2Swapper.sol#L47-L54

    function getDefaultDexOptions()
        public
        view
        override
        returns (bytes memory)
    {
        return abi.encode(block.timestamp + 1 hours);
    }

This function tries to make swap to be valid for 1 hour only, but it's not possible, because such deadline will always be valid. In order to add this restriction then specific value should be added, like: 14.07.2023 12:10:00.

So in case if data is empty and swapper is uniswap v2 and v3, then deadline protection will be disabled. Exact case, when this will be done is inside Penrose._depositFeesToYieldBox function, where no data is passed.

Tools Used

VsCode

Recommended Mitigation Steps

Make sure user always provides deadline.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L466-L507

Vulnerability details

Impact

BigBang.setBigBangConfig should accrue interests, otherwise old interests will be calculated using updated variables.

Proof of Concept

BigBang.setBigBangConfig function changes some variables that affect debt calculation for previous period of time. These are: _minDebtRate, _maxDebtRate, _debtRateAgainstEthMarket.

Because of that it means that after changing those variables, rate can be changed as well which means that interests may accrue incorrectly.

Tools Used

VsCode

Recommended Mitigation Steps

Call accrue on top of function.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/Market.sol#L197-L200

Vulnerability details

Impact

Market.setMarketConfig should accrue interests before changing _protocolFee in order to accrue previous interest with correct distribution.

Proof of Concept

When Singularity market is created then all interest is shared among lenders and protocol. Protocol receives their share that depends on protocolFee variable.

This fee can be changed any time by protocol. When it will be changed, there is no accrue call, which means that previous interest is not collected according to previous protocol fee.

Tools Used

VsCode

Recommended Mitigation Steps

In case if protocolFee is going to be changed, then accrue interests.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTLeverageModule.sol#L45-L77

Vulnerability details

Impact

BaseTOFTLeverageModule.initMultiSell will not work, because Singularity doesn't pass eth as LZ payment. As result call will revert.

Proof of Concept

User can trigger multiSell on chain 1. When BaseTOFTLeverageModule.initMultiSell is triggered on chain 1, then Singularity.multiHopSellCollateral is called(note, it doesn't send value with call) on chain 2. This will remove user's collateral and then send it for leverage to the chain 1. Pls, note that msg.value is sent to the call.

The problem here is that when BaseTOFTLeverageModule.initMultiSell was called, then it has airdrop params, which will ask LZ to send some eth to the Singularity contract(not with the call, but make transfer before call) in order to be able to pay fees. So when Singularity.multiHopSellCollateral is called by LZ, then Singularity contract will have eth in balance, but msg.value will be 0 for this call and as result leveraging will revert.

Tools Used

VsCode

Recommended Mitigation Steps

Pass address(this).balance to the call.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

TapOFT.extractTAP allows to mint more than allowed in a week.

Proof of Concept

TapOFT.extractTAP function should not allow to mint more tokens than emissionForWeek[week] in a week. https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/tokens/TapOFT.sol#L220-L229

    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");
        _mint(_to, _amount);
        mintedInWeek[week] += _amount;
        emit Minted(msg.sender, _to, _amount);
    }

As you can see there is a check that emissionForWeek[week] >= _amount, but it's not enough, as multiple calls can mint more tokens than emissionForWeek.

As TapiocaOptionBroker.exerciseOption can be called several times with more amount then emissionForWeek[week], then additional security is needed.

Once mintedInWeek[week] will be bigger than emissionForWeek[week] then emitForWeek function will revert with underflow for the next week.

Tools Used

VsCode

Recommended Mitigation Steps

Use this line. require(emissionForWeek[week] >= mintedInWeek[week] + _amount, "exceeds allowable amount");

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

In case if TapOFT.emitForWeek was not called for more than week, then tap tokens accounting will be incorrect

Proof of Concept

TapOFT contract operates in weeks. There is amount of tap tokens that can be minted for 1 week. https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/tokens/TapOFT.sol#L197-L215

    function emitForWeek() external notPaused returns (uint256) {
        require(_getChainId() == governanceChainIdentifier, "chain not valid");

        uint256 week = _timestampToWeek(block.timestamp);
        if (emissionForWeek[week] > 0) return 0;

        // Update DSO supply from last minted emissions
        dso_supply -= mintedInWeek[week - 1];

        // Compute unclaimed emission from last week and add it to the current week emission
        uint256 unclaimed = emissionForWeek[week - 1] - mintedInWeek[week - 1];
        uint256 emission = uint256(_computeEmission());
        emission += unclaimed;
        emissionForWeek[week] = emission;

        emit Emitted(week, emission);

        return emission;
    }

Let's check some points. dso_supply is the amount of tap tokens that can be minted after initial mint. No more tap should be allowed to mint. _timestampToWeek function calculates index of current week. It depends on timestamp.

So what emitForWeek function does: in case if emission was already provided, then function will revert. dso_supply is decreased by already extracted tap amount, which decreases amount that can be minted yet. In case if not all emissioned amount was extracted in previous week, then it will be added to next week emission.

As you can see, function uses week - 1 as index to find previous week. The problem with that is if TapiocaOptionBroker.newEpoch is called more than 1 week after previous call, then wrong week will be used to decrease dso_supply and to calculate not used emission.

Example. 1.TapiocaOptionBroker.newEpoch is called in week 10 so emitForWeek() is called and emissionForWeek[10] = 1000. 2.Only 100 tap was were minted in that week and no one called TapiocaOptionBroker.newEpoch for some time. TapiocaOptionBroker.newEpoch is called in week 13. 3.dso_supply -= mintedInWeek[12] will not change dso_supply, which will allow to mint more tokens than initial dso_supply 4.uint256 unclaimed = emissionForWeek[12] - mintedInWeek[12] = 0; So emissionForWeek[13] will not be increased with not used emission from previous week.

Tools Used

VsCode

Recommended Mitigation Steps

Better use same epoch approach that you have in broker and increase epoch every time, when new is started.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/option-airdrop/AirdropBroker.sol#L459 https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/option-airdrop/AirdropBroker.sol#L84

Vulnerability details

Impact

AirdropBroker mints wrong amount to 3 level users. Because of that user lose funds as they receive e18 times less than should.

Proof of Concept

AirdropBroker should mint aoTap tokens for the participants. There are 4 level of participants. For the 3 level participants, AirdropBroker mints 714 tokens instead of 714e18 tokens. Because of that 3 level participants will not receive their funds.

Tools Used

VsCode

Recommended Mitigation Steps

You need to sacel amount as it's done for level 2.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/Penrose.sol#L381-L388

Vulnerability details

Impact

Penrose.addSingularity and Penrose.addBigBang doesn't set masterContractOf variable, which then doesn't allow owner to use executeMarketFn function for markets registered in such way.

Proof of Concept

Penrose.addSingularity function allows to add already existing market to the registry. https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/Penrose.sol#L381-L388

    function addSingularity(
        address mc,
        address _contract
    ) external onlyOwner registeredSingularityMasterContract(mc) {
        isMarketRegistered[_contract] = true;
        clonesOf[mc].push(_contract);
        emit RegisterSingularity(_contract, mc);
    }

This function stores market as registered and adds market as clone of its master.

When you register market using registerSingularity function, then deploy function is called. It is a function of Penrose parent contract: BoringFactory. I will copy code without reference, because there is no node modules in this repository and @boringcrypto is dependency of this project.

function deploy(
        address masterContract,
        bytes calldata data,
        bool useCreate2
    ) public payable returns (address cloneAddress) {
        require(masterContract != address(0), "BoringFactory: No masterContract");
        bytes20 targetBytes = bytes20(masterContract); // Takes the first 20 bytes of the masterContract's address

        if (useCreate2) {
            // each masterContract has different code already. So clones are distinguished by their data only.
            bytes32 salt = keccak256(data);

            // Creates clone, more info here: https://blog.openzeppelin.com/deep-dive-into-the-minimal-proxy-contract/
            assembly {
                let clone := mload(0x40)
                mstore(clone, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
                mstore(add(clone, 0x14), targetBytes)
                mstore(add(clone, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
                cloneAddress := create2(0, clone, 0x37, salt)
            }
        } else {
            assembly {
                let clone := mload(0x40)
                mstore(clone, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
                mstore(add(clone, 0x14), targetBytes)
                mstore(add(clone, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
                cloneAddress := create(0, clone, 0x37)
            }
        }
        masterContractOf[cloneAddress] = masterContract;
        clonesOf[masterContract].push(cloneAddress);

        IMasterContract(cloneAddress).init{value: msg.value}(data);

        emit LogDeploy(masterContract, data, cloneAddress);
    }

So in the end this function set masterContractOf variable, which should mark that deployed contract has specific master.

As you can see, Penrose.addSingularity function doesn't set this masterContractOf variable.

Penrose.executeMarketFn alllows owner to execute function for markets in a loop. https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/Penrose.sol#L424-L450

    function executeMarketFn(
        address[] calldata mc,
        bytes[] memory data,
        bool forceSuccess
    )
        external
        onlyOwner
        notPaused
        returns (bool[] memory success, bytes[] memory result)
    {
        uint256 len = mc.length;
        success = new bool[](len);
        result = new bytes[](len);
        for (uint256 i = 0; i < len; ) {
            require(
                isSingularityMasterContractRegistered[
                    masterContractOf[mc[i]]
                ] || isBigBangMasterContractRegistered[masterContractOf[mc[i]]],
                "Penrose: MC not registered"
            );
            (success[i], result[i]) = mc[i].call(data[i]);
            if (forceSuccess) {
                require(success[i], _getRevertMsg(result[i]));
            }
            ++i;
        }
    }

In order to consider market valid, function checks that market's masterContractOf is registered as singularity or big bang master contract.

So in case if market was added without deploying using Penrose.addSingularity or Penrose.addBigBang functions, then owner will not be able to call market's functions using executeMarketFn.

Tools Used

VsCode

Recommended Mitigation Steps

When add singularity then set masterContractOf variable.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L336-L375

Vulnerability details

Impact

BigBang.buyCollateral can be used to steal users funds via sandwiching.

Proof of Concept

BigBang.buyCollateral function can be called by anyone who has allowance on behalf of user. https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L336-L375

    function buyCollateral(
        address from,
        uint256 borrowAmount,
        uint256 supplyAmount,
        uint256 minAmountOut,
        ISwapper swapper,
        bytes calldata dexData
    ) external notPaused solvent(from) returns (uint256 amountOut) {
        require(penrose.swappers(swapper), "SGL: Invalid swapper");


        // Let this fail first to save gas:
        uint256 supplyShare = yieldBox.toShare(assetId, supplyAmount, true);
        if (supplyShare > 0) {
            yieldBox.transfer(from, address(swapper), assetId, supplyShare);
        }


        uint256 borrowShare;
        (, borrowShare) = _borrow(from, address(swapper), borrowAmount);


        ISwapper.SwapData memory swapData = swapper.buildSwapData(
            assetId,
            collateralId,
            0,
            supplyShare + borrowShare,
            true,
            true
        );


        uint256 collateralShare;
        (amountOut, collateralShare) = swapper.swap(
            swapData,
            minAmountOut,
            from,
            dexData
        );
        require(amountOut >= minAmountOut, "SGL: not enough");


        _allowedBorrow(from, collateralShare);
        _addCollateral(from, from, false, 0, collateralShare);
    }

First thing we need to note is that allowance is checked in the end and for the check collateralShare is used, which is the amount that was provided to user's account in the end of swap. So this function sends user's USDO from yieldBox to this address. Then it borrows more amount of USDO and swaps it to collateral. There is a slippage check also, but minAmountOut is controlled by attacker and can be set to 0.

Why this function is so dangerous? Because when user provided allowance to BigBang, then anyone can call this function to send all user's USDO to BigBang and then sandwich their swap in order to earn funds. The only check that will be needed to pass is _allowedBorrow(from, collateralShare);. It will be likely not possible to make swap return 0 shares, so that means that attacker will be someone who has some allowance for user.

Example. 1.User has 1000 usdo on balance. He has max allowance for BigBang inside yieldBox. Also he provided 100 dollar borrow allowance for attacker. 2.Attacker changes price of USDO on collateral/USDO market(uniswap/curve). 3.Attacker calls buyCollateral function and sends 1000 usdo of user to BigBang he swaps this amount to less than 100 dollars and passed allowance check. 4.He closed sandwich and earn profit. 5.User lost 900 dollars.

Tools Used

VsCode

Recommended Mitigation Steps

This function should use some deviation that doesn't allow to receive much less value after the swap.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLCommon.sol#L174-L194

Vulnerability details

Impact

SGLLiquidation doesn't decrease totalCollateralShare which leads to problems when calculating provided collateral by user. Function can stop working with skim.

Proof of Concept

When user adds collateral to Singularity, then totalCollateralShare is increased. In case if user provided skim as true, that means that transfer was already done to the contract. In such case totalCollateralShare is used to determine provided amount.

When liquidation is done, then some collateral of user is swapped, however totalCollateralShare is not updated.

Because of that there will be a problems with next providing of collateral with skim as true. As totalCollateralShare is not decreased, contract will think that it holds more assets than it is in real, so function can revert, when totalCollateralShare is bigger than yieldBox.balanceOf(address(this), _assetId). https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLCommon.sol#L174-L194

    function _addTokens(
        address from,
        address to,
        uint256 _assetId,
        uint256 share,
        uint256 total,
        bool skim
    ) internal {
        bytes32 _asset_sig = _assetId == assetId ? ASSET_SIG : COLLATERAL_SIG;


        _yieldBoxShares[to][_asset_sig] += share;


        if (skim) {
            require(
                share <= yieldBox.balanceOf(address(this), _assetId) - total,
                "SGL: too much"
            );
        } else {
            yieldBox.transfer(from, address(this), _assetId, share);
        }
    }

Tools Used

VsCode

Recommended Mitigation Steps

Decrease totalCollateralShare when liquidate users.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L128-L138

Vulnerability details

Impact

emergencyWithdraw doesn't restrict depositing. Malicious user can deposit all assets again to the hacked underlying protocol.

Proof of Concept

emergencyWithdraw function is needed in order to when smth happened to underlying protocol. Each strategy has such function implemented. All they have similar pattern. They should withdraw all funds to the strategy.

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L104-L112

    function emergencyWithdraw() external onlyOwner returns (uint256 result) {
        compound("");


        uint256 toWithdraw = stEth.balanceOf(address(this));
        uint256 minAmount = (toWithdraw * 50) / 10_000; //0.5%
        result = curveStEthPool.exchange(1, 0, toWithdraw, minAmount);


        INative(address(wrappedNative)).deposit{value: result}();
    }

As you can see after the call, all funds are inside strategy(controlled by strategy). So the idea of protocol is that user's now can withdraw funds.

The problem here is that anyone can call deposit again with small amount of funds in order to send all funds back to underlying protocol.

Tools Used

VsCode

Recommended Mitigation Steps

I guess, that some variable should be set, which will not allow to deposit anymore.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/YieldBox/blob/f5ad271b2dcab8b643b7cf622c2d6a128e109999/contracts/YieldBox.sol#L196-L215

Vulnerability details

Impact

YieldBox.depositETHAsset doesn't check that user sent more funds. Those funds will stay in the contract.

Proof of Concept

YieldBox.depositETHAsset function allows to provide funds in native token. User provides amount param that should be deposited. It then wrapped and deposited.

In case if msg.value is more than amount, then this funds are not refunded to the user.

Tools Used

VsCode

Recommended Mitigation Steps

Check that msg.value == amount or return overpaid amount.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/Balancer.sol#L172-L212

Vulnerability details

Impact

Balancer.rebalance works incorrectly for native tokens. It checks that msg.value is more than amount to send, but tokens is received from mTapiocaOFT.

Proof of Concept

Balancer.rebalance function allows to send all funds that are inside mTapiocaOFT contract to similar contract on another chain. https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/Balancer.sol#L172-L212

    function rebalance(
        address payable _srcOft,
        uint16 _dstChainId,
        uint256 _slippage,
        uint256 _amount,
        bytes memory _ercData
    )
        external
        payable
        onlyOwner
        onlyValidDestination(_srcOft, _dstChainId)
        onlyValidSlippage(_slippage)
    {
        if (connectedOFTs[_srcOft][_dstChainId].rebalanceable < _amount)
            revert RebalanceAmountNotSet();


        //check if OFT is still valid
        if (
            !_isValidOft(
                _srcOft,
                connectedOFTs[_srcOft][_dstChainId].dstOft,
                _dstChainId
            )
        ) revert DestinationOftNotValid();


        //extract
        ITapiocaOFT(_srcOft).extractUnderlying(_amount);


        //send
        bool _isNative = ITapiocaOFT(_srcOft).erc20() == address(0);
        if (_isNative) {
            if (msg.value <= _amount) revert FeeAmountNotSet();
            _sendNative(_srcOft, _amount, _dstChainId, _slippage);
        } else {
            if (msg.value == 0) revert FeeAmountNotSet();
            _sendToken(_srcOft, _amount, _dstChainId, _slippage, _ercData);
        }


        connectedOFTs[_srcOft][_dstChainId].rebalanceable -= _amount;
        emit Rebalanced(_srcOft, _dstChainId, _slippage, _amount, _isNative);
    }

So first, function calls ITapiocaOFT(_srcOft).extractUnderlying(_amount), which will transfer funds to the Balancer contract. In case if token is native, then ITapiocaOFT(_srcOft) will just send this native token, so it will be in balance. But then there is a check if enough fees were paid to stargate. It checks that msg.value > _amount. The problem is that _amount wasn't sent to the contract with this call as a call value, it was received from ITapiocaOFT(_srcOft) as native transfer. Because of that msg.value likely will be less than _amount and function will revert. Balancer owner will need to send enough funds with the call in order to make it work.

Tools Used

VsCode

Recommended Mitigation Steps

Check fees in same way as for non native token.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

CompoundStrategy doesn't claim COMP rewards.

Proof of Concept

Everyone who uses compound's market is eligible for COMP rewards from Comptroller.claimComp contract. This rewards can be withdrawn and swapped to native token, which will increase staker's share.

But CompoundStrategy does nothing with it and doesn't claim COMP token. As result users lose earned funds.

Tools Used

VsCode

Recommended Mitigation Steps

You need to do that in similar way as it's done is AaveStrategy, so you claim comp tokens, swap them and deposit to compound.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Magnetar/MagnetarV2.sol#L584-L600

Vulnerability details

Impact

Some actions inside MagnetarV2.burst will not work because msg.value is used inside delegate call

Proof of Concept

Using MagnetarV2.burst user can execute several actions in a row. For each action user can provide value he would like to send.

Let's check MARKET_REMOVE_ASSET action. It will call MagnetarMarketModule.exitPositionAndRemoveCollateral function.

As you can see, some actions are executed through the module. In this case delegate call is used to call module. That means that all msg.value is sent as well.

So MagnetarMarketModule.exitPositionAndRemoveCollateral is used to remove collateral. There is one important thing here. When you remove collateral, then you have ability to withdraw it. So _withdraw function will be called. Finally we are at the problem point. This function will send all msg.value or balance as gas to LZ in case of withdrawing to another chain.

That actually means that after that we don't have balance inside MagnetarV2 and any next action that needs funds to be sent will revert.

This problem will occur for each action that needs to do withdraw and does not provide gas amount.

Tools Used

VsCode

Recommended Mitigation Steps

You need to provide action.value to all module functions, so they can't spend more.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOOptionsModule.sol#L138-L242

Vulnerability details

Impact

USDOOptionsModule.exercise doesn't send refund to user

Proof of Concept

When user would like to execute oTap, he can call USDOOptionsModule.exerciseOption which will then trigger USDOOptionsModule.exercise on another chain. So user will send some amount of USDO from one chain to another, where he wants to exercise option. That amount is credited to USDOOptionsModule contract.

Then later, ITapiocaOptionsBroker(target).exerciseOption is called and you can see that only paymentToken is sent to the function. Each user can have different discount to exercise his option and the final price will be calculated inside ITapiocaOptionsBroker.

Because of that it's possible that user will not spend all paymentTokenAmount that he sent from other chain to buy Tap tokens. But at the end of call, USDOOptionsModule.exercise function doesn't refund funds that were not spent.

Same problem exists inside BaseTOFTOptionsModule contract.

Tools Used

VsCode

Recommended Mitigation Steps

Check how much user has spent to buy tap and refund rest to his account.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLLeverage.sol#L65-L68

Vulnerability details

Impact

SGLLeverage.multiHopSellCollateral checks swapper on wrong chain

Proof of Concept

User can trigger multiSell on chain 1. When BaseTOFTLeverageModule.initMultiSell is triggered on chain 1, then Singularity.multiHopSellCollateral is called on chain 2. This will remove user's collateral and then send it for leverage to the chain 1.

Also there is a swapper check in this function.

The problem here is that this swapper is not swapper of chain 2 were it's checked now, but it is a swapper from chain 1, because swapp will be done there. As result it's not correct to do check on this chain.

Tools Used

VsCode

Recommended Mitigation Steps

Move swapper check to chain that will execute swap.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L141-L167

Vulnerability details

Impact

LidoStartegy._withdraw is likely to revert, because of not enough balance after swap. Because of that user's who deposited will never be able to withdraw. They will loose funds.

Proof of Concept

LidoStartegy._withdraw is called to withdraw eth amount from the pool. https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L141-L167

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


        uint256 queued = wrappedNative.balanceOf(address(this));
        if (amount > queued) {
            uint256 toWithdraw = amount - queued; //1:1 between eth<>stEth
            uint256 minAmount = toWithdraw - (toWithdraw * 250) / 10_000; //2.5%
            uint256 obtainedEth = curveStEthPool.exchange(
                1,
                0,
                toWithdraw,
                minAmount
            );


            INative(address(wrappedNative)).deposit{value: obtainedEth}();
        }
        queued = wrappedNative.balanceOf(address(this));
        require(queued >= amount, "LidoStrategy: not enough");


        wrappedNative.safeTransfer(to, amount);


        emit AmountWithdrawn(to, amount);
    }

As you can see in case if queued is less than amount, then strategy wants to swap toWithdraw amount of stEth to the at least minAmount which 2.5% less than toWithdraw.

After the swap whole amount is going to be swapped to user. The problem here is that in case if slippage occurs, then amount received will be likely less than toWithdraw, which means that strategy doesn't hold enough balance.

Tools Used

VsCode

Recommended Mitigation Steps

I guess that you need to exchange bigger amount of stEth and provide minimum amount to receive as toWithdraw.

Assessed type

Error

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L816-L823

Vulnerability details

Impact

There is no mechanism to track and resolve bad debt. Because of that if such happens, then USDO is not backed fully anymore.

Proof of Concept

When users borrow USDO inside BigBang, then they provide collateral to back their positions. As USDO is dollar stablecoin, that means that each USDO in circulation should have 1$ to back it inside balance of BigBang.

When collateral losses value, then position of borrower can be liquidatable. In this case liquidators should swap needed amount of user's collateral into USDO. It's possible that for some reasons, because of fast price spike or because of high transaction costs, liquidation will not be executed at time, so collateral value will be less than USDO borrowed. In such case there is no mechanism inside the protocol. You can see that it will just remove all existing collateral of user and will decrease borrow assets and supply.

Once this is done after liquidation that couldn't swap user's collateral to borrowed USDO, that means that there is no 1$ backing for each USDO token.

Tools Used

VsCode

Recommended Mitigation Steps

You can track how much USDO you lost and create ability to cover this deficit to return backing. You can cover deficit as protocol or you can create special pool, where stakers will receive some rewards in governance token(or smth else) and in case of deficit, their funds in USDO will be used to repay deficit.

Assessed type

Error