Cyfrin / 2023-07-escrow

17 stars 12 forks source link

The buyer of the Escrow contract can obtain all tokens contract balance #830

Open codehawks-bot opened 11 months ago

codehawks-bot commented 11 months ago

The buyer of the Escrow contract can obtain all tokens contract balance

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-escrow/blob/main/src/Escrow.sol#L109-L129

Summary

The buyer of the Escrow contract can obtain/get all tokens contract balance in the Escrow#resolveDispute() function. When the new Escrow is created in the EscrowFactory contract the msg.sneder is actually the buyer of the Escrow contract. This allows the msg.sender (buyer) to set the arbiter address to their own.

Vulnerability Details

Let's consider the following example:

  1. The buyer creates a new Escrow contract and sets the arbiter address to their own.
  2. Buyer call Escrow#initiateDispute() function to set the state to Disputed.
  3. The arbiter (who is actually the buyer) calls the Escrow#resolveDispute() function and passes the buyerAward parameter with the value of i_tokenContract.balanceOf(address(this)).
  4. The buyer obtains all of the tokens in the contract balance.

Impact

See Summary

Tools Used

Manual Review

Recommendations

One possible solution is to ensure that msg.sender != arbiter during the creation of a new Escrow contract.

+       if (arbiter == msg.sender) {
+           revert Improper_Action();
+       }

        Escrow escrow = new Escrow{salt: salt}(
            price,
            tokenContract,
            msg.sender, 
            seller,
            arbiter,
            arbiterFee
        );