If the token used in the Escrow contract is a ERC20 with blacklist capabilities, it can broke the whole functionality of the contract in case where the one of the actors will get blacklisted, which will get the funds be stuck in the protocol.
Vulnerability Details
There are 3 possible scenarios for this case:
Buyer gets blacklisted after the escrow was created, confirmReceipt will be working as normal, but in the case of a dispute where the seller didn't do his part of work, resolveDispute would work only if the Arbiter sets buyerAward to 0, meaning to give everything beside the fees to the Seller
Seller gets blacklisted which means that confirmReceipt would not work, so the only way funds can be taken out of the contract is by creating a dispute and Arbiter needs to set buyerAward + i_arbiterFee to be equal to the total balance of the contract, so no amount will be left to transfer to the seller.
Arbiter gets blacklisted, which is the worst of all because in the case of a dispute the funds will be stuck in the contract, since the only function callable in the Disputed state is resolveDispute, function which will revert all the time since i_arbiterFee is an immutable variable so it can be changed.
Impact
Impact is a high one since the funds could be stuck in the contract without any way of retrieving them
Tools Used
Manual review
Recommendations
Implement a Pull over Push method, which is recommended in most of the cases, by saving the amounts that every actor need to get in some storage variables, and let any of them withdraw their amount from the contract, in that sense there is no way that all the funds gets stuck in the contract
If one of the actors gets blacklisted the whole functionality will be broken and funds will be stuck in the contract
Severity
High Risk
Relevant GitHub Links
https://github.com/Cyfrin/2023-07-escrow/blob/65a60eb0773803fa0be4ba72defaec7d8567bccc/src/Escrow.sol#L119-L128
Summary
If the token used in the Escrow contract is a ERC20 with blacklist capabilities, it can broke the whole functionality of the contract in case where the one of the actors will get blacklisted, which will get the funds be stuck in the protocol.
Vulnerability Details
There are 3 possible scenarios for this case:
Buyer
gets blacklisted after the escrow was created,confirmReceipt
will be working as normal, but in the case of a dispute where the seller didn't do his part of work,resolveDispute
would work only if theArbiter
setsbuyerAward
to 0, meaning to give everything beside the fees to theSeller
Seller
gets blacklisted which means thatconfirmReceipt
would not work, so the only way funds can be taken out of the contract is by creating a dispute andArbiter
needs to setbuyerAward
+i_arbiterFee
to be equal to the total balance of the contract, so no amount will be left to transfer to the seller.Arbiter
gets blacklisted, which is the worst of all because in the case of a dispute the funds will be stuck in the contract, since the only function callable in theDisputed
state isresolveDispute
, function which will revert all the time sincei_arbiterFee
is an immutable variable so it can be changed.Impact
Impact is a high one since the funds could be stuck in the contract without any way of retrieving them
Tools Used
Manual review
Recommendations
Implement a Pull over Push method, which is recommended in most of the cases, by saving the amounts that every actor need to get in some storage variables, and let any of them withdraw their amount from the contract, in that sense there is no way that all the funds gets stuck in the contract