Tapioca DAO - chaduke's results

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. BigBang.getDebtRate() does not work for the case of _currentDebt < debtStartPoint. This is because when _currentDebt < debtStartPoint, the function will have an underflow and revert.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

BigBang.getDebtRate() will get the current debt rate.

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

However, the function does not deal with the case of _currentDebt < debtStartPoint, since the following line will revert under this case:

 uint256 debtPercentage = ((_currentDebt - debtStartPoint) *
            DEBT_PRECISION) / (_maxDebtPoint - debtStartPoint);

As a result, the function is not fully functioning for all cases.

Tools Used

VSCode

Recommended Mitigation Steps

Deal with the case of _currentDebt < debtStartPoint as follows:

function getDebtRate() public view returns (uint256) {
        if (_isEthMarket) return penrose.bigBangEthDebtRate(); // default 0.5%
        if (totalBorrow.elastic == 0) return minDebtRate;

        uint256 _ethMarketTotalDebt = BigBang(penrose.bigBangEthMarket())
            .getTotalDebt();
        uint256 _currentDebt = totalBorrow.elastic;
        uint256 _maxDebtPoint = (_ethMarketTotalDebt *
            debtRateAgainstEthMarket) / 1e18;

+       if (_currentDebut < debutSgtartPoint) return minDebtRate;
        if (_currentDebt >= _maxDebtPoint) return maxDebtRate;

        uint256 debtPercentage = ((_currentDebt - debtStartPoint) *
            DEBT_PRECISION) / (_maxDebtPoint - debtStartPoint);
        uint256 debt = ((maxDebtRate - minDebtRate) * debtPercentage) /
            DEBT_PRECISION +
            minDebtRate;

        if (debt > maxDebtRate) return maxDebtRate;

        return debt;
    }

Assessed type

Math

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/balancer/BalancerStrategy.sol#L191-L216

Vulnerability details

Impact

Detailed description of the impact of this finding. BalancerStrategy._withdraw() calculates toWithdraw as the number of shares of the pool instead of the amount of wrappedNative tokens, as a result, the function will most likely fail all the time.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

BalancerStrategy._withdraw() allows one to withdraw wrappedNative tokens from the BalanceStrategy.

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/balancer/BalancerStrategy.sol#L191-L216

There are two cases: 1) there is sufficient balance in the strategy to cover the withdrawl amount of amount; 2) the balance of wrappedNative tokens in the BalanceStrategy is insufficient to cover amount, in this case, the function will withdraw from the underlying vault the deficit.

However, when calculating the amount that needs to be withdrawn from the underlying vault, toWithdraw is calculated as number of shares of the pool instead of the amount of wrappedNative tokens. Meanwhile, function _vaultWithdraw(toWithdraw) expect toWithdraw to be the output amount of wrappedNative tokens, not the number of shares of the input.

uint256 toWithdraw = (((amount - queued) * (10 ** decimals)) /
                pricePerShare);

as a result, the value of toWithdraw could be less than it is supposed to be. As a result, even after the withdrawal from the underlying vault, there is still insufficient balance to cover the needed amount, and the function BalancerStrategy._withdraw() will likely fail all the time.

Tools Used

VSCode

Recommended Mitigation Steps

Calculate toWithdraw as the amount of wrappedNative tokens:

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

        uint256 queued = wrappedNative.balanceOf(address(this));
        if (amount > queued) {
            uint256 pricePerShare = pool.getRate();
            uint256 decimals = IStrictERC20(address(pool)).decimals();
-            uint256 toWithdraw = (((amount - queued) * (10 ** decimals)) /
-                pricePerShare);
+            uint256 toWithdraw = (amount - queued) * (10 ** decimals);
            _vaultWithdraw(toWithdraw);
        }

        require(
            amount <= wrappedNative.balanceOf(address(this)),
            "BalancerStrategy: not enough"
        );
        wrappedNative.safeTransfer(to, amount);
        updateCache();

        emit AmountWithdrawn(to, amount);
    }

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. TapOFT.extractTAP() might mint more TAP amount than allowed. This is because the allowed limit should be emissionForWeek[week] - mintedInWeek[week] instead of emissionForWeek[week]. Otherwise, a malicious or careless minter might call TapOFT.extractTAP() multiple times and mint more TAP than allowed.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

TapOFT.extractTAP() allows the minter to mint TAP but this amount should not exceed the limit.

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

However, the limit checked at L225 is emissionForWeek[week]. The correct limit should be emissionForWeek[week] - mintedInWeek[week].

As a result, a malicious or careless minter might call TapOFT.extractTAP() multiple times and mint more TAP than allowed.

My following POC code confirms my finding: the minter calls TapOFT.extractTAP() four times and as a result, minted 1876631856000000000000000 of Tap, which is 4 times of the allowed amount: 469157964000000000000000.

Please pay attention to function testTapOFT().

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "forge-std/Test.sol";
import "../contracts/tokens/TapOFT.sol";
import "../contracts/governance/twTAP.sol";



contract TapiocaTest is Test {



    function setUp() public {


    }

    function testTapOFT() public{
        address _lzEndPoint = address(111);
        address _contributors = address(112);
        address _earlySupporters = address(113);
        address _lbp = address(114);
        address _dao = address(115);
        address _airdrop = address(116);
        uint256 _governanceChainId = block.chainid;
        address _conservator = address(117);
        address _owner = msg.sender;
        TapOFT _tapoft = new TapOFT(_lzEndPoint, 
                                    _contributors,
                                    _earlySupporters,
                                    _earlySupporters,
                                    _lbp,
                                    _dao,
                                    _airdrop,
                                    _governanceChainId,
                                    _conservator
        );

        uint256 e1 = _tapoft.emitForWeek();
        console2.log("emitforWeek week 1: %d", e1);

        vm.prank(_conservator);
        _tapoft.setMinter(address(555));

        vm.startPrank(address(555));
        _tapoft.extractTAP(address(666), 469157964000000000000000);
        _tapoft.extractTAP(address(666), 469157964000000000000000);
        _tapoft.extractTAP(address(666), 469157964000000000000000);
        _tapoft.extractTAP(address(666), 469157964000000000000000);
        
        vm.stopPrank();
        console2.log("total minted for 666: %d", _tapoft.balanceOf(address(666)));
 
    }
}

Tools Used

VSCode

Recommended Mitigation Steps

Use the limit emissionForWeek[week] - mintedInWeek[week] instead:

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

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. YieldBox.depositETHAsset() fails to compare amount = msg.value, leading to 1) user A loss of funds; or 2) user B steal funds from user A.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

YieldBox.depositETHAsset() allows a user to deposit ETH into the YieldBox (and thus the strategy for the wrappedNative tokens):

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

However, the function fails to compare the specified input amount is equal to msg.value, leading to cases:

1. User A specifies the amount is smaller than msg.value. For example, amount = 1e18, and msg.value = 2e18. In this case, user A will lose 1e18 to the contract.

2. User B specifies the amount is larger than msg.value. For example, amount = 4e18, and msg.value = 3e18. Although there is a deficit, it can be filled by the the left ETH by user A. In other words, user B steals the ETH that is left by user A and deposit more than he is supposed to.

Both cases are not desirable. One need to compare msg.value == amount.

Tools Used

VSCode

Recommended Mitigation Steps

Make sure msg.value == amount so that the depositor will never gain nor loss any funds.

Assessed type

ETH-Transfer