CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Tokens will be stuck in the escrow when the state is set to disputed and the arbiter is set blacklisted in the rewards token

0xhals

Summary

Tokens will be stuck in the escrow when the state is set to disputed and the arbiter is set blacklisted in the rewards token

Vulnerability Details

  • In Escrow contract : the role of the arbiter is to solve the dispute if the seller or the arbiter initiated it.

  • When the issue state is set to disputed; there's no way for the seller or the buyer(when state is set to disputed) to receive their rewards tokens unless the arbiter solves the dispute (by calling resolveDispute function).

  • So when the arbiter calls resolveDispute function, it sets the tokens amount to be sent to the buyer, and the arbiter will get his fees i_arbiterFee and the remaining contract tokens balance (if any remains) will be sent to the seller.

  • But some tokens have a blacklist where certain accounts are prohibited from having/transferring any tokens.

  • So if the arbiter is set in the blacklist of i_tokenContract token (if it has a blacklist); then tokens will be stuck in the contract as the resolveDispute function will always revert and there's no way to change the issue state if it's set to disputed.

Impact

Rewards tokens will be stuck in the escrow contract permanently.

Proof of Concept

Line 123: i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);

  • Foundry PoC:

  1. ERC20BlackList contract (which inherits OZ ERC20Mock contract) is set to mimick the behavior of blacklisting in tokens: a simple logic is set to check if the sender or the receiver of tokens is The blacklisted account,assuming that there's only one constant blacklisted account (which is set to the arbiter address):
    add this ERC20BlackList contract file in the test/mocks directory :

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;
​
import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol";
​
contract ERC20BlackList is ERC20Mock {
constructor() ERC20Mock() {}
​
// _beforeTokenTransfer hook is updated to revert if from or to address equals the blocked address (to mimick blacklisting):
address BLACKLISTEDARBITER = address(0x50);
​
function _beforeTokenTransfer(
address from,
address to,
uint256
) internal virtual override {
if (from == BLACKLISTEDARBITER || to == BLACKLISTEDARBITER) {
revert("blacklisted");
}
}
}

  1. testResolveDisputeRevertWhenBlacklistedArbiter test is set in the EscrowTest.t.sol file to test the behavior of the escrow if the arbiter is a blacklisted address in the tokenContract (the basic arguments are copied from testDeployEscrowFromFactory test and updated to demonstrate the vulnerability).

    add this import statement and test to the EscrowTest.t.sol file :

import {ERC20BlackList} from "../mocks/ERC20BlackList.sol";
function testResolveDisputeRevertWhenBlacklistedArbiter() public {
address BLACKLISTEDARBITER = address(0x50);
// 0. tokenContract has a blacklist
ERC20BlackList tokenContract = new ERC20BlackList();
​
//1. buyer creates an escrow contract:
vm.startPrank(BUYER);
ERC20BlackList(address(tokenContract)).mint(BUYER, PRICE);
ERC20BlackList(address(tokenContract)).approve(
address(escrowFactory),
PRICE
);
escrow = escrowFactory.newEscrow(
PRICE,
tokenContract,
SELLER,
BLACKLISTEDARBITER,
ARBITER_FEE,
SALT1
);
vm.stopPrank();
​
//2. checks that the escrow contract has been deployed and the variables are set correctly:
assertEq(escrow.getPrice(), PRICE);
assertEq(address(escrow.getTokenContract()), address(tokenContract));
assertEq(escrow.getBuyer(), BUYER);
assertEq(escrow.getSeller(), SELLER);
assertEq(escrow.getArbiter(), BLACKLISTEDARBITER);
assertEq(escrow.getArbiterFee(), ARBITER_FEE);
​
//3. buyer initiates a dispute:
vm.prank(BUYER);
escrow.initiateDispute();
assertEq(uint256(escrow.getState()), uint256(IEscrow.State.Disputed));
​
//4. BLACKLISTEDARBITER tries to resolve the dispute, but it will revert:
uint256 escrowBalanceBeforeDispute = tokenContract.balanceOf(
address(escrow)
);
emit log_named_uint(
"balance before: ",
tokenContract.balanceOf(address(escrow))
);
​
vm.startPrank(BLACKLISTEDARBITER);
vm.expectRevert("blacklisted");
escrow.resolveDispute(buyerAward);
assertEq(uint256(escrow.getState()), uint256(IEscrow.State.Disputed)); // state didn't change
assertEq(
tokenContract.balanceOf(address(escrow)),
escrowBalanceBeforeDispute
); //no tokens has been transferred from the escrow
​
assertEq(tokenContract.balanceOf(BUYER), 0); //no tokens has been sent to the buyer
assertEq(tokenContract.balanceOf(BLACKLISTEDARBITER), 0); //no tokens has been sent to the BLACKLISTEDARBITER
assertEq(tokenContract.balanceOf(SELLER), 0); //no tokens has been sent to the seller
}

  1. Test result:

$ forge test --match-test testResolveDisputeRevertWhenBlacklistedArbiter
Running 1 test for test/unit/EscrowTest.t.sol:EscrowTest
[PASS] testResolveDisputeRevertWhenBlacklistedArbiter() (gas: 1594293)
Test result: ok. 1 passed; 0 failed; finished in 2.86ms

Tools Used

Manual Testing & Foundry.

Recommendations

Check (off-chain) if the arbiter is a blacklisted account in the rewards token or not before creating the escrow.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.