QA Report

[code423n4]

1. Incompatability with deflationary / fee-on-transfer tokens

• Function Cally.createVault function takes a tokenIdOrAmount parameter but this parameter is not the actual transferred amount for fee-on-transfer / deflationary (or other rebasing) tokens in case tokenType = ERC20

Impact

• The actual deposited amount might be lower than the specified depositAmount of the function parameter.
• And when users exercise or withdraw they not only receive less than expected amount but also take funds of other vaults with the same vault.token too, causes loss of funds.

Proof-of-concept

• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L200
• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L296
• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L345

Recommended Mitigation Steps

• Transfer the tokens first and compare pre-/after token balances to compute the actual amount.

2. Vault id will start from 3 not from 1.

• In Cally.createVault() function, vaultIndex += 2 is done before assigning it to vaultId. So vaultId will start from 3 and optionId will start from 4. There will not have tokenId = 0 or tokenId = 1
• This is okay, but it’s kind of abnormal. And it’s just a QA issue.

Proof of concept

• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L188-L189

Recommended Mitigation Steps

• Assign vaultIndex to vaultId before add 2.

vaultId = vaultIndex;
vaultIndex += 2;

[outdoteth]

this can be bumped to medium severity:

1. Incompatability with deflationary / fee-on-transfer tokens: Fee-on-transfer tokens are not supported #39

[HardlyDifficult]

Moved fee on transfer report to #326