search

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

2 stars 0 forks source link

Fee-on-transfer / rebasing ERC20 cannot be used as underlying #308

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L200

Vulnerability details

Some ERC20 tokens may have fee-on-transfer or change balance without owner intervention. If these tokens are used as underlying in the protocol they can be lost.

Proof of concept

Alice creates a vault with a token that has a 1% fee on transfer. She sends vault.tokenIdOrAmount = 100 during createVault, meaning that the contract balance becomes 99. When the vault is withdrawn (or someone buys and tries to exercise) the transaction fails because ERC20(vault.token).safeTransfer(msg.sender, vault.tokenIdOrAmount) (L345) tries to transfer back 100 tokens. As a consequence these tokens are irretrievable.

Recommendations

Consider if the protocol is interested in supporting these token. If not, it should be well documented to the user to not deposit them. If instead you want to support them, check token.balanceOf(address(this)) before and after a "pull" to compute the true tokenIdOrAmount.

outdoteth commented 2 years ago

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