Summary

It is stated in the readme and by Patrick Collins that the protocol will only accept vetted tokens such as USDC, DAI, WETH, etc..

Yet by using these popular tokens such as USDC, there exists a case where the funds for every participant will locked permanently. This is due to the blacklist system which is implemented by USDC and many other popular well-reputed tokens.

in Escrow.sol, if any of the participants is blacklisted (due to illegal activities, using Tornado Cash for privacy reasons, spite by one of the participants), the funds for every participant will be locked permanently.

Vulnerability Details

In the Escrow.sol contract, when an escrow is created, there are two ways to "get the funds out".

1. If there is no dispute, the buyer calls the function confirmReceipt() which will transfer the entire contract token balance to the seller address.

2. If there is a dispute, either the buyer or the seller can call initiateDispute() and the arbiter can call the function resolveDispute which will transfer part of the funds to the buyer address, a fee to the arbiter address and finally, the potential remaining funds in the contract to the seller address

If one the participant's addresses that is set to receive funds is blacklisted however, the functions will revert and essentially "brick" the contract, making it impossible to ever recover the funds.

Blacklisting is certainly not uncommon and is used many of the popular token used for payments, such as the stablecoin USDC. An address can get blacklisted for illegal activities, some were blacklisted for just using tools like Tornado Cash to keep their transactions private and it is also perfectly possible for a disgruntled participant to intentionally blacklist his address to block the withdrawal of funds.

Essentially, if any of the addresses involved is blacklisted, none of the participants can receive their funds.

We have included a POC to showcase how it works in all the cases. You can get the POC file in the following gist: https://gist.github.com/TheNaubit/b0cc2e6b4d1ae2bea637d9d89d9b5b19

To run it, has paste the content of the gist inside a file called WithdrawFailBlacklisted.t.sol inside the test folder in the project. Then run them with the following command:

Affected code:

• https://github.com/Cyfrin/2023-07-escrow/blob/fa9f20a273f1e004c1b11985ef0518b2b8ae1b10/src/Escrow.sol#L98

• https://github.com/Cyfrin/2023-07-escrow/blob/fa9f20a273f1e004c1b11985ef0518b2b8ae1b10/src/Escrow.sol#L120

• https://github.com/Cyfrin/2023-07-escrow/blob/fa9f20a273f1e004c1b11985ef0518b2b8ae1b10/src/Escrow.sol#L123

• https://github.com/Cyfrin/2023-07-escrow/blob/fa9f20a273f1e004c1b11985ef0518b2b8ae1b10/src/Escrow.sol#L127

For reference, there are other contests with similar findings like:

• https://solodit.xyz/issues/atomic-fees-delivery-susceptible-to-funds-lockout-spearbit-clober-pdf

• https://solodit.xyz/issues/h-02-dos-blacklisted-user-may-prevent-withdrawexcessrewards-code4rena-factorydao-factorydao-contest-git_

Impact

When this issue happens, all the funds in the contract are locked forever making every participant to lose their funds.

Tools Used

Manual review & Foundry

Recommendations

There are one solution with two parts:

1. Instead of trying to transfer the funds to each address, store in a state variable how many funds each address can withdraw and then create a withdraw function only callable when the escrow is finished where each participant can withdraw the funds they own. In this case, if any participant is blacklisted, at least the rest will be able to get the rest of the funds.

2. The second part of the solution is to solve the part of some participant not being able to withdraw if they are blacklisted. A solution would be to implement a function to allow each participant to set another withdrawal address for their funds, so even if they are blacklisted, they can at least withdraw those funds to another address.