code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L151-L152

Vulnerability details

Impact

User will receive more or fewer tokens than he should if using fee-on-transfer tokens.

Proof of Concept

All functions with transfer logic rely on user input to define the amount to be moved forward. If fee-on-transfer tokens are used that amount will be incorrect.

Example:

  /// @notice Allows users to deposit underlying and in the process split it into/mint 
  /// zcTokens and vault notional. Calls mPlace.mintZcTokenAddingNotional
  /// @param p Protocol Enum value associated with this market pair
  /// @param u Underlying token address associated with this market pair
  /// @param m Maturity timestamp of this associated market
  /// @param a Amount of underlying being deposited
  function splitUnderlying(uint8 p, address u, uint256 m, uint256 a) external returns (bool) {
    IErc20 uToken = IErc20(u);
    Safe.transferFrom(uToken, msg.sender, address(this), a);

    IMarketPlace mPlace = IMarketPlace(marketPlace);

    // the underlying deposit is directed to the appropriate abstraction
    if (!deposit(p, u, mPlace.cTokenAddress(p, u, m), a)) { revert Exception(6, 0, 0, address(0), address(0));
    }

    if (!mPlace.mintZcTokenAddingNotional(p, u, m, msg.sender, a)) { revert Exception(13, 0, 0, address(0), address(0));
    }

    return true;
  }

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L572-L592

  /// @notice Allows users to deposit underlying and in the process split it into/mint 
  /// zcTokens and vault notional. Calls mPlace.mintZcTokenAddingNotional
  /// @param p Protocol Enum value associated with this market pair
  /// @param u Underlying token address associated with this market pair
  /// @param m Maturity timestamp of this associated market
  /// @param a Amount of underlying being deposited
  function splitUnderlying(uint8 p, address u, uint256 m, uint256 a) external returns (bool) {
    IErc20 uToken = IErc20(u);
    Safe.transferFrom(uToken, msg.sender, address(this), a);

    IMarketPlace mPlace = IMarketPlace(marketPlace);

    // the underlying deposit is directed to the appropriate abstraction
    if (!deposit(p, u, mPlace.cTokenAddress(p, u, m), a)) { revert Exception(6, 0, 0, address(0), address(0));
    }

    if (!mPlace.mintZcTokenAddingNotional(p, u, m, msg.sender, a)) { revert Exception(13, 0, 0, address(0), address(0));
    }

    return true;
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Check difference in balance between and after transfer to account for the correct amount of tokens.

JTraversa commented 2 years ago

Duplicate of #63

bghughes commented 2 years ago

Duplicate of #63

0xean commented 2 years ago

marking as dupe of #143 wardens QA report.

code-423n4 / 2022-07-swivel-findings