codehawks-2023-07-codehawks-escrow-contract-l01

[L-01] Constructor of Escrow should make sure that buyer, seller, arbiter are different from each other

보고서

Summary

구매자/판매자/중재자가 각각 동일한 주소인지 확인하지 않는다. 특히 중재자가 어느 한쪽과 동일한 주소로 셋팅되면 편파적인 중재를 할 수 있다.

Keyword

input validation, business logic vul

Vulnerability

• src/Escrow.sol#L32-L52

Escrow 생성자에서 구매자와 판매자, 중재자가 동일 주소인지 확인하지 않는다. 중재자를 구매자 또는 판매자와 동일한 주소로 설정한다면 편파적인 중재를 할 수 있다. 따라서 최소한 이들이 각각 동일 주소로 설정되는 것은 막아야 한다.

    constructor(
        uint256 price,
        IERC20 tokenContract,
        address buyer,
        address seller,
        address arbiter,
        uint256 arbiterFee
    ) {
        if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
        if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
        if (seller == address(0)) revert Escrow__SellerZeroAddress();
        if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
        if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
        i_price = price;
        i_tokenContract = tokenContract;
 
        // @audit - it does not check if buyer != sellers and buyer != arbiter
        i_buyer = buyer;
        i_seller = seller; 
        i_arbiter = arbiter;
        i_arbiterFee = arbiterFee;
    }

Impact

구매자와 판매자, 중재자가 각각 동일하다면 악의적으로 이용할 수 있다. 편파적인 중재를 할 수 있다.

Mitigation

    constructor(
        uint256 price,
        IERC20 tokenContract,
        address buyer,
        address seller,
        address arbiter,
        uint256 arbiterFee
    ) {
        if (address(tokenContract) == address(0)) revert Escrow__TokenZeroAddress();
        if (buyer == address(0)) revert Escrow__BuyerZeroAddress();
        if (seller == address(0)) revert Escrow__SellerZeroAddress();
        if (arbiterFee >= price) revert Escrow__FeeExceedsPrice(price, arbiterFee);
        if (tokenContract.balanceOf(address(this)) < price) revert Escrow__MustDeployWithTokenBalance();
        i_price = price;
        i_tokenContract = tokenContract;
 
        // @audit - it does not check if buyer != sellers and buyer != arbiter
+       if (buyer == arbiter) revert Escrow__InvalidAddress();
+       if (seller == arbiter) revert Escrow__InvalidAddress();
+       if (buyer == seller) revert Escrow__InvalidAddress();
 
        i_buyer = buyer;
        i_seller = seller; 
        i_arbiter = arbiter;
        i_arbiterFee = arbiterFee;
    }

---

tags: bughunting, codehawks, smart contract, solidity, lack-of-input-validation-vul, business-logic-vul, severity low