Clever Mocha Pheasant - Incomplete Token Permission Validation in Collateral Checks

[sherlock-admin2]

Clever Mocha Pheasant

Medium

Incomplete Token Permission Validation in Collateral Checks

Summary

The CollateralManager contract contains a vulnerability in its collateral validation logic where it fails to verify token spending permissions. The _checkBalance function only validates token balances while ignoring crucial allowance checks, which can lead to failed transactions and inconsistent contract state.

The vulnerability manifests in the ERC20 validation logic where the function performs an incomplete verification:

https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/CollateralManager.sol#L576

if (collateralType == CollateralType.ERC20) {
    return
        _collateralInfo._amount <=
        IERC20(_collateralInfo._collateralAddress).balanceOf(
            _borrowerAddress
        );
}

This implementation naively assumes that having sufficient balance is the only requirement for collateral deposit. However, in ERC20's design, both balance and allowance are required for token transfers. The impact is significant as it creates a false sense of security where collateral validation passes but the actual deposit operation fails due to insufficient allowance.

This design flaw ripples through the system because a successful balance check leads users and integrating protocols to believe the collateral can be deposited. However, the subsequent depositAndDeploy operation will revert when attempting to execute safeTransferFrom, resulting in failed transactions and wasted gas.

Recommended mitigation steps

The validation logic should be enhanced to verify both balance and allowance for ERC20 tokens, ensuring that all requirements for successful collateral deposit are met:

if (collateralType == CollateralType.ERC20) {
    IERC20 token = IERC20(_collateralInfo._collateralAddress);
    uint256 amount = _collateralInfo._amount;
    
    // Check both balance and allowance
    return amount <= token.balanceOf(_borrowerAddress) && 
           amount <= token.allowance(_borrowerAddress, address(this));
}

This solution provides comprehensive validation by ensuring that:

1. The borrower has sufficient token balance
2. The CollateralManager has permission to transfer the required amount
3. The deposit operation will have all necessary conditions met

Similar validation could be extended to ERC721 and ERC1155 tokens, though these are less critical as approvals can be handled separately:

if (collateralType == CollateralType.ERC721) {
    IERC721Upgradeable nft = IERC721Upgradeable(_collateralInfo._collateralAddress);
    return _borrowerAddress == nft.ownerOf(_collateralInfo._tokenId) &&
           (nft.isApprovedForAll(_borrowerAddress, address(this)) ||
            nft.getApproved(_collateralInfo._tokenId) == address(this));
}

Through these changes, the contract provides reliable validation that accurately reflects all requirements for successful collateral deposit, improving user experience and system reliability while preventing unnecessary gas consumption from failed transactions.