Tapioca DAO - Nyx's results

The first ever Omnichain money market, powered by LayerZero.

General Information

Platform: Code4rena

Start Date: 05/07/2023

Pot Size: $390,000 USDC

Total HM: 136

Participants: 132

Period: about 1 month

Judge: LSDan

Total Solo HM: 56

Id: 261

League: ETH

Tapioca DAO

Findings Distribution

Researcher Performance

Rank: 66/132

Findings: 2

Award: $256.59

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: peakbolt

Also found by: Ack, Nyx, carrotsmuggler, n1punp, rvierdiiev

Labels

bug
3 (High Risk)
satisfactory
duplicate-1168

Awards

254.4518 USDC - $254.45

External Links

Lines of code

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

Vulnerability details

Impact

Liquidators can lose rewards.

Proof of Concept

function liquidate(
        address[] calldata users,
        uint256[] calldata maxBorrowParts,
        ISwapper swapper,
        bytes calldata collateralToAssetSwapData
    ) external notPaused {

The liquidate() function can be used to liquidate multiple users at once. And there is a swap inside this process, so there is a slippage parameter that is collateralToAssetSwapData. It is used inside the _liquidateUser() function as we can see in the below.

uint256 minAssetMount = 0;
        if (_dexData.length > 0) {
            minAssetMount = abi.decode(_dexData, (uint256)); 
        }

The problem is when the liquidator liquidates more than one user, there is only one slippage parameter(minAssetAmount) and it's used for all liquidations. And that slippage amount can be low for the second liquidation or third. The liquidator can lose his rewards due to slippage being too low.

Tools Used

Manual Review

A separate slippage parameter needs to be used in every liquidation.

Assessed type

Other

#0 - c4-pre-sort

2023-08-05T12:54:34Z

minhquanym marked the issue as duplicate of #122

#1 - c4-judge

2023-09-21T12:34:05Z

dmvt marked the issue as satisfactory

Awards

2.1417 USDC - $2.14

Labels

bug
2 (Med Risk)
satisfactory
duplicate-163

External Links

Lines of code

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

Vulnerability details

Impact

Malicious users can harm protocol by using the withdrawAllMarketFees() function with no slippage parameter.

Proof of Concept

/// @notice Loop through the master contracts and call `_depositFeesToYieldBox()` to each one of their clones.
    /// @dev `swappers_` can have one element that'll be used for all clones. Or one swapper per MasterContract.
    /// @dev Fees are withdrawn in TAP and sent to the FeeDistributor contract
    /// @param markets_ Singularity &/ BigBang markets array
    /// @param swappers_ one or more swappers to convert the asset to TAP.
    /// @param swapData_ swap data for each swapper
    function withdrawAllMarketFees(
        IMarket[] calldata markets_,
        ISwapper[] calldata swappers_,
        IPenrose.SwapData[] calldata swapData_
    ) public notPaused {
        require(
            markets_.length == swappers_.length &&
                swappers_.length == swapData_.length,
            "Penrose: length mismatch"
        );
        require(address(swappers_[0]) != address(0), "Penrose: zero address");
        require(address(markets_[0]) != address(0), "Penrose: zero address");

        _withdrawAllProtocolFees(swappers_, swapData_, markets_); 

        emit ProtocolWithdrawal(markets_, block.timestamp);
    }

Anyone can call the withdrawAllMarketFees() function and send fees to yieldbox but the problem is, there is a swap inside the _depositFeesToYieldBox, as we can see below.

ISwapper.SwapData memory swapData = swapper.buildSwapData(
                assetId,
                wethAssetId,
                0,
                feeShares,
                true,
                true
            );
            (amount, ) = swapper.swap(
                swapData,
                dexData.minAssetAmount, //@audit user input swapData !
                feeTo,
                ""
            );

If a malicious user calls the function with no slippage parameter(swapData), the protocol can lose fees due to slippage.

Tools Used

Manual Review

Access control on the withdrawAllMarketFees() function will solve the problem.

Assessed type

Access Control

#0 - c4-pre-sort

2023-08-06T06:06:22Z

minhquanym marked the issue as duplicate of #266

#1 - c4-judge

2023-09-19T11:43:43Z

dmvt marked the issue as duplicate of #245

#2 - c4-judge

2023-09-22T22:16:22Z

dmvt marked the issue as satisfactory

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter