40,000 USDC
View results
Submission Details
Severity: low

Repeated invocation of `confirmReceipt` function in `Escrow.sol` contract

Summary

The confirmReceipt function in the Escrow contract is protected by the inState(State.Created) modifier, which reverts if the function is called when the contract is not in the Created state. However, if a malicious actor gains control over the buyer's address, they could potentially call this function more than once before the state changes, leading to unexpected behavior.

Vulnerability Details

The confirmReceipt function is designed to confirm the receipt of the service by the buyer. It is protected by the inState(State.Created) modifier, which ensures that the function can only be called when the contract is in the Created state. However, if a malicious actor gains control over the buyer's address, they could potentially call this function more than once before the state changes. This could lead to unexpected behavior, as the function transfers the entire balance of the contract to the seller.

Code Snippet

function confirmReceipt() external onlyBuyer inState(State.Created) {
s_state = State.Confirmed;
emit Confirmed(i_seller);
i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this)));
}

Impact

If a malicious actor gains control over the buyer's address and calls the confirmReceipt function more than once before the state changes, it could lead to unexpected behavior. However, given the atomic nature of transactions in Ethereum, the impact of this issue is likely to be low. The state of the contract should change to Confirmed after the first call to confirmReceipt, preventing further calls from having any effect.

Tools Used

Manual code review

Recommendations

To mitigate this potential issue, it is recommended to add additional checks in the confirmReceipt function to ensure that it can only be called once. This could be implemented by adding a boolean state variable that tracks whether the function has been called, and checking this variable at the beginning of the function, like so:

bool private receiptConfirmed = false;
function confirmReceipt() external onlyBuyer inState(State.Created) {
require(!receiptConfirmed, "Receipt has already been confirmed");
receiptConfirmed = true;
s_state = State.Confirmed;
emit Confirmed(i_seller);
i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this)));
}

This would ensure that the confirmReceipt function cannot be called more than once, thereby preventing the potential issues described above.

Support

FAQs

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