code-423n4 / 2022-05-cally-findings

2 stars 0 forks source link

No check for deflationary/fee-on-transfer tokens #282

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

Cally.sol#L158-L201 Cally.sol#L258-L297 Cally.sol#L318-L346

Vulnerability details

Impact

If the underlying asset for a vault is a fee-on-transfer ERC20 token then the actual amount received by the contract could be less than the value of tokenIdOrAmount of the vault.

If the contract does not have enough tokens to cover the difference then the exercise and withdraw functions would not work since the safeTransfer function at the end of exercise and withdraw would revert due to an insufficient balance.

This could lead to vault owners being unable to access their funds until the contract has enough tokens and option holders might be unable to exercise their option before it expires, leading to lost funds.

Proof of Concept

Unable to withdraw:

  1. Create a vault using a fee-on-transfer token as the underlying asset. For this instance, the Cally.sol contract holds none of these tokens before the initial transfer into the contract.
  2. At the end of createVault, the amount of vault.tokenIdOrAmount tokens is transferred from the owner of the vault to the Cally.sol contract.
  3. After the transfer, the contract has a balance less than vault.tokenIdOrAmount of the tokens.
  4. When trying to withdraw the tokens, after calling initiateWithdraw, the withdraw function reverts when the safeTransfer function is called.

Unable to exercise:

  1. Buy an option from a vault with fee-on-transfer ERC20 tokens as the underlying asset.
  2. When calling exercise, the safeTransfer at the end of the function will fail if the contract does not have enough tokens. The option holder will not be able to exercise the option.

Recommended Mitigation Steps

Consider checking the contract's balance before and after ERC20 transfers in createVault. The vault.tokenIdOrAmount could be set to the difference of the balances.

outdoteth commented 2 years ago

reference issue: https://github.com/code-423n4/2022-05-cally-findings/issues/39