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

Remove Non-Reentrant Modifier from Arbitrated Dispute Resolution Function

hunter_w3b

Summary

In the Escrow.sol contract, the resolveDispute function is restricted to be called only by the onlyArbiter modifier, implying that the arbiter is trusted and will not execute reentrancy attacks. As such, the use of the nonReentrant modifier in this context is unnecessary and can be removed to save gas.

Vulnerability Details

The resolveDispute function in the Escrow.sol contract is designed to handle dispute resolution and distribute awards accordingly. It is guarded by the onlyArbiter modifier, which ensures that only an authorized arbiter can execute this function. Given that the arbiter is assumed to be trusted, the need for the nonReentrant modifier can be questioned, as it guards against reentrancy attacks, which are not expected from a trusted arbiter.

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

function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
uint256 tokenBalance = i_tokenContract.balanceOf(address(this));
uint256 totalFee = buyerAward + i_arbiterFee; // Reverts on overflow
if (totalFee > tokenBalance) {
revert Escrow__TotalFeeExceedsBalance(tokenBalance, totalFee);
}
​
s_state = State.Resolved;
emit Resolved(i_buyer, i_seller);
​
if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, buyerAward);
}
if (i_arbiterFee > 0) {
i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);
}
tokenBalance = i_tokenContract.balanceOf(address(this));
if (tokenBalance > 0) {
i_tokenContract.safeTransfer(i_seller, tokenBalance);
}
}

Support

FAQs

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