Tapioca DAO - bin2chen's results

Lines of code

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

Vulnerability details

Impact

Users can skip the allowedBorrow() restriction and use other people's assets

Proof of Concept

in BigBang.addCollateral(), will determine if the operator has enough allowances The code is as follows.

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


    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;
        }
    }

The problem is that addCollateral() supports two parameters, share and amount. The user can pass share == 0, but amount has a value. Since share == 0, the allowedBorrow(from, share) restriction can be skipped.

but inside the method it reassigns share via amount.

addCollateral() -> _addCollateral()

    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, collateralId, share, oldTotalCollateralShare, skim);
        emit LogAddCollateral(skim ? address(yieldBox) : from, to, share);
    }

This results in the fact that anyone can pass in shares=0, but amount >0 to call addCollateral() to use the assets of from

Note:SGLCollateral.addCollateral() has a similar problem.

Tools Used

Recommended Mitigation Steps

Move _allowedBorrow() to inside _addCollateral()

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

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

incorrectly calculating callerReward

Proof of Concept

The _liquidateUser() code is implemented as follows.

    function _liquidateUser(
        address user,
        uint256 maxBorrowPart,
        ISwapper swapper,
        uint256 _exchangeRate,
        bytes calldata _dexData
    ) private {
        if (_isSolvent(user, _exchangeRate)) return;

        (
            uint256 startTVLInAsset,
            uint256 maxTVLInAsset
        ) = _computeMaxAndMinLTVInAsset(
                userCollateralShare[user],
                _exchangeRate
            );
@>      uint256 callerReward = _getCallerReward(
            userBorrowPart[user],
            startTVLInAsset,
            maxTVLInAsset
        );

....

Use the _getCallerReward() method to calculate the callerReward. But there is a problem with the first parameter of this method, the input is userBorrowPart[user], which is the shares value. But _getCallerReward() asks for assets value, which should be converted using totalBorrow.toElastic(). So you need to convert shares to assets before passing in _getCallerReward().

The first parameter of _getCallerReward() is assets when needed.

    function _getCallerReward(
@>      uint256 borrowed,
        uint256 startTVLInAsset,
        uint256 maxTVLInAsset
    ) internal view returns (uint256) {
        if (borrowed == 0) return 0;
        if (startTVLInAsset == 0) return 0;

@>      if (borrowed < startTVLInAsset) return 0;
        if (borrowed >= maxTVLInAsset) return minLiquidatorReward;

        uint256 rewardPercentage = ((borrowed - startTVLInAsset) *
            FEE_PRECISION) / (maxTVLInAsset - startTVLInAsset);

        int256 diff = int256(minLiquidatorReward) - int256(maxLiquidatorReward);
        int256 reward = (diff * int256(rewardPercentage)) /
            int256(FEE_PRECISION) +
            int256(maxLiquidatorReward);

        return uint256(reward);
    }

Tools Used

Recommended Mitigation Steps

    function _liquidateUser(
        address user,
        uint256 maxBorrowPart,
        ISwapper swapper,
        uint256 _exchangeRate,
        bytes calldata _dexData
    ) private {
        if (_isSolvent(user, _exchangeRate)) return;

        (
            uint256 startTVLInAsset,
            uint256 maxTVLInAsset
        ) = _computeMaxAndMinLTVInAsset(
                userCollateralShare[user],
                _exchangeRate
            );
        uint256 callerReward = _getCallerReward(
-           userBorrowPart[user],
+           totalBorrow.toElastic(userBorrowPart[user],false),
            startTVLInAsset,
            maxTVLInAsset
        );

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/governance/twTAP.sol#L499

Vulnerability details

Impact

_claimRewardsOn() does not check whether have duplicate tokens in the _rewardTokens[] array, resulting in duplicate tokens that can be passed in to steal rewards

Proof of Concept

Users can retrieve rewards via the following paths

tapOFT.claimRewards(tokenID,rewardTokens)->_nonblockingLzReceive() (layerzero) -> tapOFT._claimRewards(rewardTokens) ->twTap.claimAndSendRewards(rewardTokens)

Above the path, the user can specify which rewardTokens to get back.

The actual final calculation is done in TwTAP.claimAndSendRewards().

Current implementation:

TwTAP.claimAndSendRewards() -> TwTAP._claimRewardsOn()

    function _claimRewardsOn(
        uint256 _tokenId,
        address _to,
@>      IERC20[] memory _rewardTokens
    ) internal {
@>      uint256[] memory amounts = claimable(_tokenId);
        unchecked {
            uint256 len = _rewardTokens.length;
            for (uint256 i = 0; i < len; ) {
                uint256 claimableIndex = rewardTokenIndex[_rewardTokens[i]];
                uint256 amount = amounts[i];

                if (amount > 0) {
                    // Math is safe: `amount` calculated safely in `claimable()`
@>                  claimed[_tokenId][claimableIndex] += amount;
@>                  rewardTokens[claimableIndex].safeTransfer(_to, amount);
                }
                ++i;
            }
        }
    }

From the above we can see that first we get the array of claimable amounts, then we loop to accumulate the total and modify claimed[_tokenId][claimableIndex]. amounts is a temporary variable that stays constant in the loop.

One problem here is that there is no determination of whether _rewardTokens has duplicate tokens if the same token is passed in maliciously it can duplicate the claim. For example, if you pass in _rewardTokens = [Token_A,Token_A,Token_A], then you can repeat the claim Token_A 3 times. A malicious user can pass in repeated rewardTokens, until the token is drained.

Tools Used

Recommended Mitigation Steps

Add duplicate rewardTokens judgment.

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/BaseTOFT.sol#L540

Vulnerability details

Impact

Using BaseTOFTOptionsModule.exercise() with the wrong signature prevents the execution of PT_TAP_EXERCISE

Proof of Concept

In BaseTOFT.sol, BaseTOFTOptionsModule.exercise.selector is called if the type is PT_TAP_EXERCISE.

contract BaseTOFT is BaseTOFTStorage, ERC20Permit {
....

    function _nonblockingLzReceive(
        uint16 _srcChainId,
        bytes memory _srcAddress,
        uint64 _nonce,
        bytes memory _payload
    ) internal virtual override {
...

        } else if (packetType == PT_TAP_EXERCISE) {
            _executeOnDestination(
                Module.Options,
                abi.encodeWithSelector(
@>                  BaseTOFTOptionsModule.exercise.selector,
                    _srcChainId,
                    _srcAddress,
                    _nonce,
                    _payload
                ),
                _srcChainId,
                _srcAddress,
                _nonce,
                _payload
            );
        }   

This call is wrong, the first parameter module is missing Let's look at the actual arguments to BaseTOFTOptionsModule.exercise()

contract BaseTOFTOptionsModule is BaseTOFTStorage {
....
    function exercise(
@>      address module,
        uint16 _srcChainId,
        bytes memory _srcAddress,
        uint64 _nonce,
        bytes memory _payload
    ) public {
        (
            ,
            ITapiocaOptionsBrokerCrossChain.IExerciseOptionsData
                memory optionsData,
            ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData
                memory tapSendData,
            ICommonData.IApproval[] memory approvals
        ) = abi.decode(
                _payload,

This causes the call to PT_TAP_EXERCISE to fail forever, which is a cross-chain operation, and if the task can't be executed, the corresponding asset can't be operated either.

Tools Used

Recommended Mitigation Steps

contract BaseTOFT is BaseTOFTStorage, ERC20Permit {
....

    function _nonblockingLzReceive(
        uint16 _srcChainId,
        bytes memory _srcAddress,
        uint64 _nonce,
        bytes memory _payload
    ) internal virtual override {
...

        } else if (packetType == PT_TAP_EXERCISE) {
            _executeOnDestination(
                Module.Options,
                abi.encodeWithSelector(
                    BaseTOFTOptionsModule.exercise.selector,
+                   optionsModule,                    
                    _srcChainId,
                    _srcAddress,
                    _nonce,
                    _payload
                ),
                _srcChainId,
                _srcAddress,
                _nonce,
                _payload
            );
        }   

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

if _currentDebt < debtStartPoint will cause getDebtRate() to underflow , and anything else that relies on this method will fail

Proof of Concept

in BigBang.sol, you can configure debtStartPoint and this value will be used in getDebtRate().

The code is as follows. in init() , to configure

    function init(bytes calldata data) external onlyOnce {
        (
...
        _isEthMarket = collateralId == penrose.wethAssetId();
        if (!_isEthMarket) {
            debtRateAgainstEthMarket = _debtRateAgainstEth;
            maxDebtRate = _debtRateMax;
            minDebtRate = _debtRateMin;
@>          debtStartPoint = _debtStartPoint;
        }

        minLiquidatorReward = 1e3;
        maxLiquidatorReward = 1e4;
        liquidationBonusAmount = 1e4;
        borrowOpeningFee = 50; // 0.05%
        liquidationMultiplier = 12000; //12%
    }        

in getDebtRate(), this value participates in the calculation of debtPercentage.

    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 (_currentDebt >= _maxDebtPoint) return maxDebtRate;

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

Calculation of _currentDebt will perform (_currentDebt - debtStartPoint) if _currentDebt < debtStartPoint will underflow The logical way to do this is to return minDebtRate if _currentDebt <= debtStartPoint.

Tools Used

Recommended Mitigation Steps

    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 (_currentDebt >= _maxDebtPoint) return maxDebtRate;
+       if (_currentDebt <= debtStartPoint) return minDebtRate;

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

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

currentBalance() Incorrect calculation

Proof of Concept

StargateStrategy.currentBalance() The code is as follows:

    function _currentBalance() internal view override returns (uint256 amount) {
        uint256 queued = wrappedNative.balanceOf(address(this));
@>      (amount, ) = lpStaking.userInfo(lpStakingPid, address(this));
        uint256 claimableRewards = compoundAmount();
        return amount + queued + claimableRewards;
    }

The value returned is : amount + queued + claimableRewards

queued = the wrappedNative balance of the current contract claimableRewards = the amount of wrappedNative after rewards conversion

but amount is the balance of the lpToken

https://etherscan.io/address/0xB0D502E938ed5f4df2E681fE6E419ff29631d62b#code

contract LPStaking is Ownable {
...
    struct UserInfo {
@>      uint256 amount; // How many LP tokens the user has provided.
        uint256 rewardDebt; // Reward debt. See explanation below.   
    }

This requires converting the lpToken count to the wrappedNative count in order to add them up. Conversion can be done using _stgEthPool.amountLPtoLD()

Tools Used

Recommended Mitigation Steps

    function _currentBalance() internal view override returns (uint256 amount) {
        uint256 queued = wrappedNative.balanceOf(address(this));
        (amount, ) = lpStaking.userInfo(lpStakingPid, address(this));
+       amount = _stgEthPool.amountLPtoLD(amount);        
        uint256 claimableRewards = compoundAmount();
        return amount + queued + claimableRewards;
    }

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/bcf61f79464cfdc0484aa272f9f6e28d5de36a8f/contracts/tOFT/modules/BaseTOFTOptionsModule.sol#L241

Vulnerability details

Impact

Missing incoming gas when executing ISendFrom(tapSendData.tapOftAddress).sendFrom(), resulting in possible failure to execute exerciseInternal().

Proof of Concept

in BaseTOFTOptionsModule.exerciseInternal() if tapSendData.withdrawOnAnotherChain == true will execute ISendFrom(tapSendData.tapOftAddress).sendFrom()

    function exerciseInternal(
        address from,
        uint256 oTAPTokenID,
        address paymentToken,
        uint256 tapAmount,
        address target,
        ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData
            memory tapSendData,
        ICommonData.IApproval[] memory approvals
    ) public {
...
        if (tapSendData.withdrawOnAnotherChain) {
@>          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 {
            IERC20(tapSendData.tapOftAddress).safeTransfer(from, tapAmount);
        }
    }    

sendFrom() does not work because the current call does not take gas with it, and should use ISendFrom(tapSendData.tapOftAddress).sendFrom{value: address(this).balance}().

Tools Used

Recommended Mitigation Steps

    function exerciseInternal(
        address from,
        uint256 oTAPTokenID,
        address paymentToken,
        uint256 tapAmount,
        address target,
        ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData
            memory tapSendData,
        ICommonData.IApproval[] memory approvals
    ) public {
...
        if (tapSendData.withdrawOnAnotherChain) {
-          ISendFrom(tapSendData.tapOftAddress).sendFrom(
+          ISendFrom(tapSendData.tapOftAddress).sendFrom{ value: address(this).balance }(
                address(this),
                tapSendData.lzDstChainId,
                LzLib.addressToBytes32(from),
                tapAmount,
                ISendFrom.LzCallParams({
                    refundAddress: payable(from),
                    zroPaymentAddress: tapSendData.zroPaymentAddress,
                    adapterParams: LzLib.buildDefaultAdapterParams(
                        tapSendData.extraGas
                    )
                })
            );
        } else {
            IERC20(tapSendData.tapOftAddress).safeTransfer(from, tapAmount);
        }
    }    

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/TapiocaOptionLiquidityProvision.sol#L237

Vulnerability details

Impact

TapiocaOptionLiquidityProvision.sol no way to retrieve assets earned from yieldBox.

Proof of Concept

in TapiocaOptionLiquidityProvision.sol lockPosition.amount and activeSingularities[_singularity].totalDeposited record assets value

    function lock(
        address _to,
        IERC20 _singularity,
        uint128 _lockDuration,
        uint128 _amount
    ) external returns (uint256 tokenId) {
...
        require(_lockDuration > 0, "tOLP: lock duration must be > 0");
        require(_amount > 0, "tOLP: amount must be > 0");

        uint256 sglAssetID = activeSingularities[_singularity].sglAssetID;
        require(sglAssetID > 0, "tOLP: singularity not active");

        // Transfer the Singularity position to this contract
        uint256 sharesIn = yieldBox.toShare(sglAssetID, _amount, false);

        yieldBox.transfer(msg.sender, address(this), sglAssetID, sharesIn);
@>      activeSingularities[_singularity].totalDeposited += _amount;

        // Mint the tOLP NFT position
        tokenId = ++tokenCounter;
        _safeMint(_to, tokenId);

        // Create the lock position
        LockPosition storage lockPosition = lockPositions[tokenId];
        lockPosition.lockTime = uint128(block.timestamp);
        lockPosition.sglAssetID = uint128(sglAssetID);
        lockPosition.lockDuration = _lockDuration;
@>      lockPosition.amount = _amount;

        emit Mint(_to, uint128(sglAssetID), lockPosition);
    }    

Unlocking also only takes the assets amount deposited at that time.

    function unlock(
        uint256 _tokenId,
        IERC20 _singularity,
        address _to
    ) external returns (uint256 sharesOut) {
        require(_exists(_tokenId), "tOLP: Expired position");

        LockPosition memory lockPosition = lockPositions[_tokenId];
        //@audit low ? 这个是不是>会好点，等于getLock()是有效的
        require(
            block.timestamp >=
                lockPosition.lockTime + lockPosition.lockDuration,
            "tOLP: Lock not expired"
        );
        require(
            activeSingularities[_singularity].sglAssetID ==
                lockPosition.sglAssetID,
            "tOLP: Invalid singularity"
        );

        require(
            _isApprovedOrOwner(msg.sender, _tokenId),
            "tOLP: not owner nor approved"
        );

        _burn(_tokenId);
        delete lockPositions[_tokenId];

        // Transfer the tOLR tokens back to the owner
        sharesOut = yieldBox.toShare(
            lockPosition.sglAssetID,
@>          lockPosition.amount,
            false
        );

        yieldBox.transfer(
            address(this),
            _to,
            lockPosition.sglAssetID,
@>          sharesOut
        );
        activeSingularities[_singularity].totalDeposited -= lockPosition.amount;

        emit Burn(_to, lockPosition.sglAssetID, lockPosition);
    }

We know from the above code that when lock(), lockPosition.amount is the value of assets at that time. when unlock() only retrieves the assets recorded at that time: lockPosition.amount.

Then the profit stored in the yieldBox for that period of time is not distributed, it stays in the contract.

Tools Used

Recommended Mitigation Steps

lockPosition.amount and activeSingularities[_singularity].totalDeposited It makes more sense to record shares corresponding to yieldBox.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

in extractTAP() , without mintedInWeek[week] be added to the calculation, leading to a possible exceeding of the emissionForWeek[week]

Proof of Concept

extractTAP() The code is as follows:

    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);
    }

extractTAP() It needs to be judged that it can't be more than emissionForWeek[week]

But the current use of the require(missionForWeek[week] >= _amount) restriction is wrong, and doesn't take mintedInWeek[week] into account This causes TapiocaOptionBroker to potentially take more than emissionForWeek[week]. Correctly should use:require(emissionForWeek[week] - mintedInWeek[week] >= _amount)

Tools Used

Recommended Mitigation Steps

    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

Context

Lines of code

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

Vulnerability details

Impact

Due to the PHASE_3_AMOUNT_PER_USER wrong value, the PHASE 3 user can only get a small quantity of 714.

Proof of Concept

AirdropBroker PHASE 3 Each user get a fixed amount PHASE_3_AMOUNT_PER_USER the code：

contract AirdropBroker is Pausable, BoringOwnable, FullMath {
....

    /// =====-------======
    ///      Phase 3
    /// =====-------======

@>  uint256 public constant PHASE_3_AMOUNT_PER_USER = 714;
    uint256 public constant PHASE_3_DISCOUNT = 50 * 1e4;


    function _participatePhase3(
        bytes calldata _data
    ) internal returns (uint256 oTAPTokenID) {
        uint256 _tokenID = abi.decode(_data, (uint256));

        require(PCNFT.ownerOf(_tokenID) == msg.sender, "adb: Not eligible");
        address tokenIDToAddress = address(uint160(_tokenID));
        require(
            userParticipation[tokenIDToAddress][3] == false,
            "adb: Already participated"
        );
        // Close eligibility
        // To avoid a potential attack vector, we cast token ID to an address instead of using _to,
        // no conflict possible, tokenID goes from 0 ... 714.
        userParticipation[tokenIDToAddress][3] = true;

        uint128 expiry = uint128(lastEpochUpdate + EPOCH_DURATION); // Set expiry to the end of the epoch
@>      uint256 eligibleAmount = PHASE_3_AMOUNT_PER_USER;
        uint128 discount = uint128(PHASE_3_DISCOUNT);
@>      oTAPTokenID = aoTAP.mint(msg.sender, expiry, discount, eligibleAmount);
    }

From the above code, we can see that PHASE_3_AMOUNT_PER_USER value is wrong, currently only '714 wei', the correct should be: 'PHASE_3_AMOUNT_PER_USER = 714 * 1e18' instead of only '714 wei' similar to PHASE_2_AMOUNT_PER_USER : uint256 bleAmount = uint256 (PHASE_2_AMOUNT_PER_USER [_role]) * 1e18;

Tools Used

Recommended Mitigation Steps

contract AirdropBroker is Pausable, BoringOwnable, FullMath {
....

-   uint256 public constant PHASE_3_AMOUNT_PER_USER = 714;
+   uint256 public constant PHASE_3_AMOUNT_PER_USER = 714 * 1e18;
    uint256 public constant PHASE_3_DISCOUNT = 50 * 1e4;

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

Decrease in totalCollateralShare is not handled correctly, resulting in _addCollateral() possibly not working properly

Proof of Concept

In BigBang.sol, totalCollateralShare represents the current total share When userCollateralShare[user] increases, it needs to increase. When userCollateralShare[user] decreases, it needs to be decreased.

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


    function _removeCollateral(
        address from,
        address to,
        uint256 share
    ) internal {
@>      userCollateralShare[from] -= share;
@>      totalCollateralShare -= share;
        emit LogRemoveCollateral(from, to, share);
        yieldBox.transfer(address(this), to, collateralId, share);
    }    

In _updateBorrowAndCollateralShare() also reduces userCollateralShare[user] but forgets to reduce the totalCollateralShare.

_liquidateUser()->_updateBorrowAndCollateralShare()

    function _updateBorrowAndCollateralShare(
        address user,
        uint256 maxBorrowPart,
        uint256 _exchangeRate
    )
...

@>      userCollateralShare[user] -= collateralShare;
        require(borrowAmount != 0, "SGL: solvent");

        totalBorrow.elastic -= uint128(borrowAmount);
        totalBorrow.base -= uint128(borrowPart);
    }

This leads to the problem that if a user is liquidated, totalCollateralShare will be larger than the correct value.

_addCollateral(skim = true) may not work correctly, as this method will compare totalCollateralShare

_addCollateral() -> _addTokens()

    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, collateralId, share, oldTotalCollateralShare, skim);
        emit LogAddCollateral(skim ? address(yieldBox) : from, to, share);
    }

    function _addTokens(
        address from,
        uint256 _tokenId,
        uint256 share,
@>      uint256 total,
        bool skim
    ) internal {
        if (skim) {
            require(
@>              share <= yieldBox.balanceOf(address(this), _tokenId) - total,
                "BigBang: too much"
            );
        } else {
            yieldBox.transfer(from, address(this), _tokenId, share);
        }
    }

Tools Used

Recommended Mitigation Steps

    function _updateBorrowAndCollateralShare(
        address user,
        uint256 maxBorrowPart,
        uint256 _exchangeRate
    )
...

        userCollateralShare[user] -= collateralShare;
+       totalCollateralShare -= collateralShare;        
        require(borrowAmount != 0, "SGL: solvent");

        totalBorrow.elastic -= uint128(borrowAmount);
        totalBorrow.base -= uint128(borrowPart);
    }

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/convex/ConvexTricryptoStrategy.sol#L130

Vulnerability details

Impact

compoundAmount() Does not include balances already in the contract, resulting in inaccuracies

Proof of Concept

Stargate's LPStaking mechanism is that : when execute deposit() and it will trigger claim rewards

The following code comes from https://etherscan.io/address/0xB0D502E938ed5f4df2E681fE6E419ff29631d62b#code

contract LPStaking is Ownable {
...
    function deposit(uint256 _pid, uint256 _amount) public {
        PoolInfo storage pool = poolInfo[_pid];
        UserInfo storage user = userInfo[_pid][msg.sender];
        updatePool(_pid);
        if (user.amount > 0) {
            uint256 pending = user.amount.mul(pool.accStargatePerShare).div(1e12).sub(user.rewardDebt);
@>          safeStargateTransfer(msg.sender, pending);
        }
        pool.lpToken.safeTransferFrom(address(msg.sender), address(this), _amount);

This means that StargateStrategy.sol will increase the stgTokenReward balance of the contract as long as there is a deposit().

But the current compoundAmount() doesn't contain this stgTokenReward balance that is already in the contract

    function compoundAmount() public view returns (uint256 result) {
        uint256 claimable = lpStaking.pendingStargate(
            lpStakingPid,
            address(this)
        );
        result = 0;
        if (claimable > 0) {
            ISwapper.SwapData memory swapData = swapper.buildSwapData(
                address(stgTokenReward),
                address(wrappedNative),
                claimable,
                0,
                false,
                false
            );
            result = swapper.getOutputAmount(swapData, "");

            result = result - (result * 50) / 10_000; //0.5%
        }
    }

From the above implementation we can see that only pendingStargate() is included, not the balance already in the contract.

So compoundAmount() is much less than the actual value.

Tools Used

Recommended Mitigation Steps

Include the balance already in the contract

    function compoundAmount() public view returns (uint256 result) {
        uint256 claimable = lpStaking.pendingStargate(
            lpStakingPid,
            address(this)
        );

+       claimable += stgTokenReward.banlanceOf(address(this));

        result = 0;
        if (claimable > 0) {
            ISwapper.SwapData memory swapData = swapper.buildSwapData(
                address(stgTokenReward),
                address(wrappedNative),
                claimable,
                0,
                false,
                false
            );
            result = swapper.getOutputAmount(swapData, "");

            result = result - (result * 50) / 10_000; //0.5%
        }
    }

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/TapiocaOptionBroker.sol#L230

Vulnerability details

Impact

Execution of tOLP.transferFrom() incorrectly uses msg.sender as the from, which prevents authorized operators from executing participate().

Proof of Concept

TapiocaOptionBroker.participate() The current code implementation is as follows:

    function participate(
        uint256 _tOLPTokenID
    ) external returns (uint256 oTAPTokenID) {
        // Compute option parameters
        (bool isPositionActive, LockPosition memory lock) = tOLP.getLock(
            _tOLPTokenID
        );
        require(isPositionActive, "tOB: Position is not active");
        require(lock.lockDuration >= EPOCH_DURATION, "tOB: Duration too short");

        TWAMLPool memory pool = twAML[lock.sglAssetID];

        require(
@>          tOLP.isApprovedOrOwner(msg.sender, _tOLPTokenID),
            "tOB: Not approved or owner"
        );

        // Transfer tOLP position to this contract
@>      tOLP.transferFrom(msg.sender, address(this), _tOLPTokenID);

        uint256 magnitude = computeMagnitude(
            uint256(lock.lockDuration),
            pool.cumulative
        );

The current protocol mechanism is that if _tOLPTokenID authorizes an operator (isApproved), he does not have to be the owner to perform participate().

require( tOLP.isApprovedOrOwner(msg.sender, _tOLPTokenID), "tOB: Not approved or owner" );

But in tOLP.transferFrom(), it uses: from=msg.sender, since the operator is not the owner, tOLP.transferFrom() will revert.

This results in the operator not being able to execute participate() even though he is authorized (isApproved).

The correct from should use tOLP.ownerOf(_tOLPTokenID)

Tools Used

Recommended Mitigation Steps

    function participate(
        uint256 _tOLPTokenID
    ) external returns (uint256 oTAPTokenID) {
        // Compute option parameters
        (bool isPositionActive, LockPosition memory lock) = tOLP.getLock(
            _tOLPTokenID
        );
        require(isPositionActive, "tOB: Position is not active");
        require(lock.lockDuration >= EPOCH_DURATION, "tOB: Duration too short");

        TWAMLPool memory pool = twAML[lock.sglAssetID];

        require(
            tOLP.isApprovedOrOwner(msg.sender, _tOLPTokenID),
            "tOB: Not approved or owner"
        );

        // Transfer tOLP position to this contract

-       tOLP.transferFrom(msg.sender, address(this), _tOLPTokenID);       
+       tOLP.transferFrom(tOLP.ownerOf(_tOLPTokenID), address(this), _tOLPTokenID);

Assessed type

Context