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

2 stars 0 forks source link

Upgraded Q -> M from 95 [1654474439349] #332

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Judge has assessed an item in Issue #95 as Medium risk. The relevant finding follows:

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.

HardlyDifficult commented 2 years ago

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