sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

ctf_sec - Fully exercise option to receive fully eligible amount via TOFTOptionsReceiverModule may result in loss of fund #82

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

ctf_sec

medium

Fully exercise option to receive fully eligible amount via TOFTOptionsReceiverModule may result in loss of fund

Summary

Fully exercise option to receive fully eligible amount via TOFTOptionsReceiverModule may result in loss of fund

Vulnerability Detail

Fully exercise option to receive fully eligible amount may result in loss of fund

https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L162

address(this).safeApprove(address(pearlmit), _options.paymentTokenAmount);

            /// @dev call exerciseOption() with address(this) as the payment token
            uint256 bBefore = balanceOf(address(this)); // @audit should be payment token
            ITapiocaOptionBroker(_options.target).exerciseOption(
                _options.oTAPTokenID,
                address(this), //payment token
                _options.tapAmount
            );
            address(this).safeApprove(address(pearlmit), 0); // Clear approval
            uint256 bAfter = balanceOf(address(this));

Line of code

  1. https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L162
  2. https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L203
  3. https://github.com/Tapioca-DAO/tap-token/blob/df77a5c4502f30e2b9f7eadb3fdc532d9e738cd2/contracts/options/TapiocaOptionBroker.sol#L394

    Proof of concept

    In TOFTOptionsReceiverModule.sol, look at this logic:

Line of code

address(this).safeApprove(address(pearlmit), _options.paymentTokenAmount);

            /// @dev call exerciseOption() with address(this) as the payment token
            uint256 bBefore = balanceOf(address(this)); // @audit should be payment token
            ITapiocaOptionBroker(_options.target).exerciseOption(
                _options.oTAPTokenID,
                address(this), //payment token
                _options.tapAmount
            );
            address(this).safeApprove(address(pearlmit), 0); // Clear approval
            uint256 bAfter = balanceOf(address(this));

then in the end the TAP token is transferred to user. Line of code

} else {
      //send on this chain
      IERC20(tapOft).safeTransfer(_options.from, _options.tapAmount);
  }

Impact

note that exercise option is user provide liquidity with payment token and receive TAP token at discount rate.

If the tap amonut passed is 0, the code will fill in the available chosen amount. Line of code

if (netAmount == 0) revert NoLiquidity();
        uint256 eligibleTapAmount = muldiv(tOLPLockPosition.ybShares, gaugeTotalForEpoch, netAmount);
        eligibleTapAmount -= oTAPCalls[_oTAPTokenID][cachedEpoch]; // Subtract already exercised amount
        if (eligibleTapAmount < _tapAmount) revert TooHigh();

        uint256 chosenAmount = _tapAmount == 0 ? eligibleTapAmount : _tapAmount;
        if (chosenAmount < 1e18) revert TooLow();
        oTAPCalls[_oTAPTokenID][cachedEpoch] += chosenAmount; // Adds up exercised amount to current epoch

        // Finalize the deal
        _processOTCDeal(_paymentToken, paymentTokenOracle, chosenAmount, oTAPPosition.discount);

note the line of code

     uint256 chosenAmount = _tapAmount == 0 ? eligibleTapAmount : _tapAmount;

then the eligibleTapAmount of TAP will be transferred to user

So if user pass in .tapAmount as 0, he wants to receive the full amount of TAP token, when exercise option, the full eligible amount of transfered to the contract,

but in the end

     IERC20(tapOft).safeTransfer(_options.from, _options.tapAmount);

_options.tapAmount is 0 so user receives 0 token

Impact

.

Code Snippet

  1. https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L162

  2. https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L203

  3. https://github.com/Tapioca-DAO/tap-token/blob/df77a5c4502f30e2b9f7eadb3fdc532d9e738cd2/contracts/options/TapiocaOptionBroker.sol#L394

Tool used

Manual Review

Recommendation

When the .tapAmount is 0,

the contract should default to transferring the eligibleTapAmount

instead of the specified .tapAmount.

anther way to handle is transfer the available balance of tap token to user

  uint256 balance =   IERC20(tapOft).balanceOf(address(this))
  IERC20(tapOft).safeTransfer(_options.from, balance);

Duplicate of #130