Escrow contract deployed with parameters such as seller, buyer, and arbiter addresses. But the issue is contract distributes tokens using these addresses. It's dangerous because if one of the addresses gets blacklisted (if payment tokens have a blacklist like USDT or USDC), whole contract payments will block, and seller and arbiter will lose their funds and buyer tokens will be stuck in the contract.
Vulnerability Details
this function is for arbiter to resolve the dispute:
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) { //@audit G- Consider checking for (!= 0) because its cheaper
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);
}
}
we see that tokens transfer to stored addresses, so if one of these addresses is blacklisted, no one can get any tokens. That's a problem because in this scenario there is no chance to recover tokens from the contract.
Tools Used
manually
Impact
If payment token have blacklist and one of the addresses gets blacklisted other user payments will lose too. (Like USDT/USDC)
Recommendations
Consider implementing a separate claim function to avoid this issue.
Transferring tokens directly to stored addresses is problematic for payments
Severity
High Risk
Relevant GitHub Links
https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L94-L99
https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L109-L129
Summary
Escrow contract deployed with parameters such as seller, buyer, and arbiter addresses. But the issue is contract distributes tokens using these addresses. It's dangerous because if one of the addresses gets blacklisted (if payment tokens have a blacklist like USDT or USDC), whole contract payments will block, and seller and arbiter will lose their funds and buyer tokens will be stuck in the contract.
Vulnerability Details
this function is for arbiter to resolve the dispute:
we see that tokens transfer to stored addresses, so if one of these addresses is blacklisted, no one can get any tokens. That's a problem because in this scenario there is no chance to recover tokens from the contract.
Tools Used
manually
Impact
If payment token have blacklist and one of the addresses gets blacklisted other user payments will lose too. (Like USDT/USDC)
Recommendations
Consider implementing a separate claim function to avoid this issue.