083.md

PawelK

medium

Withdraw collateral doesn't use _amount parameter to transfer ERC20 collateral

Summary

Withdraw collateral doesn't use _amount parameter to transfer ERC20 collateral, and instead always sends full collateral amount.

Vulnerability Detail

The issue lies in the CollateralEscrowV1.sol contract, specifically in the withdraw() and _withdrawCollateral() functions. The issue is that the withdraw() function does not use the _amount parameter to transfer the ERC20 collateral, but instead always sends the full collateral amount, which is stored in the Collateral struct under the _collateral._amount field.

This creates a potential inconsistency within the contract state, which could be exploited when combined with other vulnerabilities. An attacker could potentially trigger a situation where the contract is left in an inconsistent state, allowing them to perform unauthorized actions or cause unexpected behavior.

Impact

Potential inconsistent state inside the CollateralEscrowV1.sol which might be exploited combined with other vulnerability. For example, because of no guard on commitColleral we can modify collateral info, set amount to 1 wei on ERC20 token, and withdraw all collateral.

Code Snippet

/**
     * @notice Withdraws a collateral asset from the escrow.
     * @param _collateralAddress The address of the collateral contract.
     * @param _amount The amount to withdraw.
     * @param _recipient The address to send the assets to.
     */
    function withdraw(
        address _collateralAddress,
        uint256 _amount,
        address _recipient
    ) external virtual onlyOwner {
        require(_amount > 0, "Withdraw amount cannot be zero");
        Collateral storage collateral = collateralBalances[_collateralAddress];
        require(
            collateral._amount >= _amount,
            "No collateral balance for asset"
        );
        _withdrawCollateral(
            collateral,
            _collateralAddress,
            _amount,
            _recipient
        );
        collateral._amount -= _amount;
        emit CollateralWithdrawn(_collateralAddress, _amount, _recipient);
    }
    
    /**
     * @notice Internal function for transferring collateral assets out of this contract.
     * @param _collateral The collateral asset to withdraw.
     * @param _collateralAddress The address of the collateral contract.
     * @param _amount The amount to withdraw.
     * @param _recipient The address to send the assets to.
     */
    function _withdrawCollateral(
        Collateral memory _collateral,
        address _collateralAddress,
        uint256 _amount,
        address _recipient
    ) internal {
        // Withdraw ERC20
        if (_collateral._collateralType == CollateralType.ERC20) {
            IERC20Upgradeable(_collateralAddress).transfer(
                _recipient,
                _collateral._amount // described issue: `_amount` should be passed here instead
            );
        }
        ...

Tool used

Manual Review

Recommendation

Use _amount here instead of _collateral._amount