Lines of code

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

Vulnerability details

Impact

Some ERC20 tokens have a fee on each transfer. The protocol doesn’t handle the fee when transferring this kind of ERC20 tokens, leading to the inconsistent amount of token actually received in the contract. This will cause users to fail to exercise options due to not enough tokens in the protocol, or the protocol will lose more tokens to compensate users for exercising options.

Proof of Concept

In the createVault function, the amount of token is recorded in tokenIdOrAmount. However, if the ERC20(vault.token) collects a fee on every transfer. The actual amount of token received is not equal to tokenIdOrAmount. vault.tokenIdOrAmount will record more amount of token than it received.

        Vault memory vault = Vault({
            tokenIdOrAmount: tokenIdOrAmount,
            ...
        });

        ...

        // transfer the NFTs or ERC20s to the contract
        vault.tokenType == TokenType.ERC721
            ? ERC721(vault.token).transferFrom(msg.sender, address(this), vault.tokenIdOrAmount)
            : ERC20(vault.token).safeTransferFrom(msg.sender, address(this), vault.tokenIdOrAmount);

Tools Used

vim

Recommended Mitigation Steps

Use balanceAfter - balanceBefore rather than using amount directly:

function retrieveTokens(address sender, uint256 amount) public {
 uint256 balanceBefore = deflationaryToken.balanceOf(address(this));
 deflationaryToken.transferFrom(sender, address(this), amount);
 uint256 balanceAfter = deflationaryToken.balanceOf(address(this));
 amount = balanceAfter.sub(balanceBefore);
 totalTokenTransferred += amount;
}

Token Transfer Calculation Accuracy in this article: https://blog.chain.link/defi-security-best-practices/

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