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

Buyers may lose their funds

mahyar

Vulnerability Details

In the Escrow contract buyers lose their tokens if they dispute with seller and if there is no arbiter.
Because if they disputed and there is no arbiter, only choice of buyer is to call confirmReceipt function to send tokens to user or they can leave it behind to get locked in contract forever.

Recommendations

What you can do is if there is no arbiter, in initiateDispute function update the s_state variable with new value of enum called DisputedWithNoArbiter and also define a new state variable called deadline and update it to 30 days or any time that you see work best. here is how it should look like:

  1. create new variable in state uint64 private deadline;

  2. add new value called DisputedWithNoArbiter in State enum

  3. change initiateDispute function to this

function initiateDispute() external onlyBuyerOrSeller inState(State.Created) {
if (i_arbiter == address(0)){
s_state = State.DisputedWithNoArbiter;
deadline = block.timestamp + 30 days;
} else {
s_state = State.Disputed;
}
emit Disputed(msg.sender);
}

After you need to create new function called confirmDispute, this should be only callable by seller address and when s_state is equal to DisputedWithNoArbiter to allow buyer get back their tokens and you should add new value in State enum called DisputeConfirmed that could be set if seller called this function, you can do it like this:

  1. add new value called DisputeConfirmed in State enum

  2. implement new function called confirmDispute() like this

function confirmDispute() external inState(State.DisputedWithNoArbiter){
if(msg.sender != i_seller){
revert Escrow__OnlySeller(); // make sure to add this error too
}
​
s_state = State.DisputeConfirmed;
​
emit ConfirmedDispute(msg.sender); // you can also add this event
}

After all you should create new function called executeDispute() thats only callable by buyer and it checks to see if dispute is confirmed by seller or deadline is passed to allow buyer get their tokens back.

(Note: the reason you need deadline is if the seller doesn't confirm the dispute buyer still can withdraw tokens by a delay, you can also give 2% of total balance to seller if he/she confirmed the dispute, it's up to you)

  1. implement new function called executeDispute() like this

function executeDispute() external onlyBuyer{
// dispute should be confirmed by seller or deadline should be passed
if(s_state != State.DisputeConfirmed && block.timestamp < deadline){
revert Escrow__NotAllowedToExecute(); // make sure to add this error too
}
​
s_state = State.Resolved;
emit Resolved(i_buyer, i_seller);
​
uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
i_tokenContract.safeTransfer(i_buyer, tokenBalance);
}

By donig it this way quality of the code will be a step further than the current implementation and buyers don't lose funds if they dispute with sellers.

(Also make sure to add the new functions to your interface)

Support

FAQs

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