Lines of code

Vulnerability details

C4 issue

ADD-03: Some ERC20 can revert on a zero value transfer

Comment

Some ERC20 can revert on zero value transfer, and in the original implementation sometimes this is not checked. For example in AutoCompound::withdrawBalances

function withdrawBalances(address[] calldata tokens, address to) external override nonReentrant {
        if (msg.sender != withdrawer) {
            revert Unauthorized();
        }
        uint256 i;
        uint256 count = tokens.length;
        for (; i < count; ++i) {
            uint256 balance = positionBalances[0][tokens[i]];
            _withdrawBalanceInternal(0, tokens[i], to, balance, balance);
        }
    }
function _withdrawBalanceInternal(uint256 tokenId, address token, address to, uint256 balance, uint256 amount)
        internal
    {
        require(amount <= balance, ""amount>balance"");
        positionBalances[tokenId][token] = positionBalances[tokenId][token] - amount;
        emit BalanceRemoved(tokenId, token, amount);
        SafeERC20.safeTransfer(IERC20(token), to, amount);
        emit BalanceWithdrawn(tokenId, token, to, amount);
    }

If one of the balance is zero, then it will revert.

Mitigation

PR #28 The mitigation code checks if transfer amount >0 before transfer, for example:

function withdrawBalances(address[] calldata tokens, address to) external override nonReentrant {
        if (msg.sender != withdrawer) {
            revert Unauthorized();
        }
        uint256 i;
        uint256 count = tokens.length;
        uint256 balance;
        address token;
        for (; i < count; ++i) {
            token = tokens[i];
            balance = positionBalances[0][token];
            if (balance != 0) {
                _withdrawBalanceInternal(0, token, to, balance, balance);
            }
        }
    }

No need to add this check everytime because there are cases when we can just let the code revert. For example, if someone tries to withdraw with amount=0, then revert is also OK. The mitigation solved the orginal issue

Conclusion

LGTM

code-423n4 / 2024-04-revert-mitigation-findings