Submission Details
Severity:
Functions calling contracts/addresses with transfer hooks are missing reentrancy guards
Summary
Functions calling contracts/addresses with transfer hooks are missing reentrancy guards
Vulnerability Details
Even if a function follows the best practice of check-effects-interaction, not using a reentrancy guard when there may be transfer hooks will open users of this protocol up to read-only reentrancy attacks, which cannot be prevented except by block-listing the whole protocol.
There are 2 instances of this issue:
File: src/Escrow.sol
// @audit `confirmReceipt()`
98: i_tokenContract.safeTransfer(i_seller, i_tokenContract.balanceOf(address(this)));
File: src/EscrowFactory.sol
// @audit `newEscrow()`
39: tokenContract.safeTransferFrom(msg.sender, computedAddress, price);
Impact
Users of this protocol are vulnerable to read-only reentrancy attacks
Tools Used
Manual review
Recommendations
Use nonReentrant modifiers like you've used here:
function resolveDispute(uint256 buyerAward) external onlyArbiter nonReentrant inState(State.Disputed) {
//some code
//
if (buyerAward > 0) {
i_tokenContract.safeTransfer(i_buyer, buyerAward);
}
if (i_arbiterFee > 0) {
i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee);//@audit if i_arbiter is address0, what will happen here?
}
tokenBalance = i_tokenContract.balanceOf(address(this));
if (tokenBalance > 0) {
i_tokenContract.safeTransfer(i_seller, tokenBalance);
}
}
Prize pool breakdown
Total prize
40,000 USDC
nSLOC
186215 USDC / LOC
37,000 USDC
3,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.