40,000 USDC
View results
Submission Details
Severity: medium
Valid

Should follow pull over push pattern when sending tokens. If any of `sender`,`receiver`,`arbiter` is blacklisted then `confirmReceipt()` or `resolveDispute()` might be DoSed.

Summary

confirmReceipt() or resolveDispute will release amount owed to seller,buyer,arbiter.

If any of the transfer fails, e.g USDC blacklisting, a whole transaction will revert.

Vulnerability Details

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))); //@audit - pull over push
}
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); //@audit - pull over push
}
if (i_arbiterFee > 0) {
i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee); //@audit - pull over push
}
tokenBalance = i_tokenContract.balanceOf(address(this)); //@audit - pull over push
if (tokenBalance > 0) {
i_tokenContract.safeTransfer(i_seller, tokenBalance);
}

Impact

confirmReceipt() or resolveDispute will be DoSed if any of the seller,buyer,arbiter is blacklisted.

Tools Used

Manual Review

Recommendations

Should have claim() function where buyer, seller, arbiter would be able to withdraw their claims.

+ mapping(address => uint256) public claimable;
+ function claim() external {
+ uint256 amount = claimable[msg.sender];
+ if (amount > 0)
+ {
+ delete claimable[msg.sender];
+ i_tokenContract.safeTransfer(msg.sender, amount);
+ }
+ }
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))); //@audit - pull over push
+ claimable[i_seller] = price;
}
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); //@audit - pull over push
- }
- if (i_arbiterFee > 0) {
- i_tokenContract.safeTransfer(i_arbiter, i_arbiterFee); //@audit - pull over push
- }
- tokenBalance = i_tokenContract.balanceOf(address(this));
- if (tokenBalance > 0) {
- i_tokenContract.safeTransfer(i_seller, tokenBalance); //@audit - pull over push
- }
+ claimable[i_seller] = price - totalFee;
+ claimable[i_buyer] = buyerAward;
+ claimable[i_arbiter] = i_arbiterFee;

Support

FAQs

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