Report #15579

Allowing reentering into Pipeline can cause loss of assets

BIC Response

The BIC determined that this report outlines expected functionality given that the user must call an "untrusted" contract in order for there to be any potential loss of funds.

Pipeline was built with the philosophy that it is not the smart contract's role to protect users against misuse. See the Risk section of the Pipeline whitepaper: https://evmpipeline.org/pipeline.pdf#section.6

Due to these reasons, we are closing the submission and no reward will be issued.

Reporter Response

given that the user must call an "untrusted" contract

No, users don't have to "explicitly" call an untrusted contract. It's possible that the untrusted contract is called indirectly. For example, a user calls the OpenSea protocol, the OpenSea protocol sends an NFT to a buyer, then the buyer reenters Pipeline.

In such a case, the user doesn't trust the buyer, but why would they execute trades on OpenSea? Because they know OpenSea is designed so that even if the buyer is untrusted, the seller's funds are well-protected as long as the protocol behaves as their expectation, i.e., trade some token for NFT. Imagine a scenario where OpenSea has a reentrancy vulnerability and some users were attacked because of this, should the user take the entire responsibility? Does it belong to the category of user's "misuse" because the buyer is untrusted? (I disagree. It should be OpenSea's responsibility.)

If OpenSea is secure only when users trade with those they trust, its usage is greatly limited. Similarly, suppose Pipeline was designed to let users interact with protocols only they trust, with additional constraints that those protocols cannot interact with any other parties the user does not trust. In that case, the usage of Pipeline is significantly limited, which I believe is not what you designed Pipeline for.

In your previous response, you said Pipeline does not protect users against misuse, which I also agree with. However, is it true that this kind of reentrancy attack must be caused by a user's misuse? I agree that a user "directly" calling an untrusted contract is the user's fault, but how about an indirect call, as I mentioned above? To ensure that we are on the same page, let me provide a concrete example for my previous comment:

1. The user use Pipeline to execute three actions: (1) Loads an NFT to Pipeline (2) Loads some USDC to Pipeline (3) Fulfills a buy order on OpenSea with the NFT (to receive more USDC) (4) Trades USDC on Uniswap to DAI and send back to themselves.
2. The malicious buyer reenters Pipeline in step (3) and steals the user's USDC

In this example, which steps from the users do you consider a "misuse" of Pipeline?

1. If calling OpenSea's fulfill function but sending NFT to an untrusted buyer is a misuse:

I don't think this is a misuse. First, the user is using OpenSea in an OpenSea's expected way. OpenSea does not have the additional requirement that the buyer should be someone trusted. If this function call is not a misuse from OpenSea's perspective, what causes it becomes a misuse if doing this in Pipeline?

2. If putting USDC in Pipeline before calling OpenSea is a misuse:

Then you should update your documentation and explicitly say that not only leaving assets between transactions would cause a loss of funds but within a single transaction between actions too. The Pipeline whitepaper (https://evmpipeline.org/pipeline.pdf#section.6), Point 1, and the Pipeline code verified on Etherscan (https://etherscan.io/address/0xb1bE0000bFdcDDc92A8290202830C4Ef689dCeaa#code), Line 14, only mentions assets can be stolen between transactions (i.e., after the victim's transaction ends). They didn't say assets can be stolen within a victim's transaction. Users can be attacked if they think this risk will only happen between transactions, but as I proved, it's possible to steal assets within a single transaction.

3. If calling any protocol that will call any untrusted party is a misuse:

Then you should also update your documentation to warn users. At the time of writing, this risk is not written in the whitepaper's Risk section (https://evmpipeline.org/pipeline.pdf#section.6) as the "majority of known risks". However, this is a risk that can happen in a fairly common use case of DeFi protocols (this is not an exhaustive list):

a. OpenSea and other NFT marketplaces, since they all can trigger the receiving hook on the potentially untrusted buyer.

b. AMMs that can send raw ETH to potentially untrusted users such as Uniswap and Curve.

c. Generally, any protocol that can unwrap WETH to ETH and sends it to potentially untrusted users.

d. Not a protocol but any ERC777 token since they have built-in transfer hooks.

As I mentioned above, if this risk of assets being stolen within a single transaction is what a user should prevent when using Pipeline, the use case of Pipeline would be significantly restricted.

BIC Response

We believe this edge case is defended against in the following manner:

• It is expected that users transfer assets back out of Pipeline at the end of a call (known as "Unloading" Pipeline). Unloading should be performed such that the amount returned is the amount expected to be received based on the nature of the transaction.
• In the case where a malicious contract tries to steal assets during a transaction, a proper Unloading step would cause Pipeline to revert.

In practice, this means that the Pipeline user should (1) set safeguards (Unloading) if they don't trust the contract being called or (2) don't set safeguards to reduce gas costs.

This is intentional design decision and based on the above discussion we still believe it is true that any loss of funds (within or between transactions) requires misuse of Pipeline.

Reporter Response

I do agree that unloading assets from Pipeline as the last step can mitigate this issue in some of the use cases, but not all. As you mentioned, users can ensure the amount returned is the amount expected to protect themselves, which I also agree with. However, some DeFi protocols design their APIs with an option of "maximum balance", e.g., withdraw/swap with the maximum balance of the user. This design is especially common in yield strategies or vaults such as Yearn. If the user unloads assets from Pipeline in this kind of "maximum balance" way, they can still be exploited and receive less than expected since there is no expected/minimum amount check.

Of course, one can argue that users should avoid this kind of "maximum balance" operation to prevent them from being hacked. However, knowing the exact amount to receive before the transaction executes is not reasonably practical in common DeFi use cases. One of the most popular operations in DeFi, swap tokens via DEXs, does not guarantee to receive an "exact" amount since the spot price fluctuates. Suppose users want to include a swap during a Pipeline multi-call. In that case, it's highly likely that they cannot know or control the final receiving amount and, therefore, cannot protect themselves using the way you mentioned. (Note that DEXs often provide the "minimum received amount", but it doesn't help in this situation since any assets left in the Pipeline after the user's transaction can be retrieved by anyone).

As a result, if users want to protect themselves by ensuring the exact received amount, they almost (if not always) need to avoid swapping assets on DEXs in their Pipeline calls. Do you consider this an intended limitation of a use case of Pipeline?

BIC Response

This is the purpose of Clipboard:

Clipboard allows users to Copy return values stored as returnData from any AdvancedPipeCalls that have already been executed and Paste them into the callData of the next AdvancedPipeCall, in a customizable manner.

A client can either: (1) Grab the return value from a swap (2) add a balanceOf() call after a swap

And then paste that value into the unload step. This allows users to protect themselves by ensuring they receive the expected amount, or the expected amount above some threshold.

Reporter Response

I'm aware of the usage of Clipboard, but it's unclear how using it in either of the ways you mentioned can help users to ensure receiving the expected amount (and thus avoid being attacked).

A client can either: (1) Grab the return value from a swap (2) add a balanceOf() call after a swap

Method (1): Say the user swaps 1000 USDC to DAI, and the return should be 1000 DAI. Under an attack, the attacker steals 500 USDC before the swap, so the return value from a swap becomes 500. The user then can only unload 500 DAI.

Method (2): In the same scenario of an attack, after the swap, the user calls DAI.balanceOf(address(Pipeline)) and knows that Pipeline has 500 DAI, so they unload 500 DAI.

The key point is: Pipeline does not have built-in logic to let the user "ensure" if they have at least some number of tokens. The user can know the swap return value, as you mentioned, but how can they check it against an expected value? Can you elaborate more on this?

BIC Response

A proper unloading of Pipeline involves Piping asset transfer function calls to transfer assets to desired destinations. Clients can unload a specified amount of assets from Pipeline (based on return values of a previous call as mentioned above, or based on anything of their choosing), and if that step fails, the entire transaction reverts.

Reporter Response

Let me provide a more detailed example of why the defense methods you mentioned do not work. This example is the same as the previous except the user swaps USDC to WETH in the second last step:

1. The user uses Pipeline to execute five actions: (1) Loads an NFT to Pipeline (2) Loads some USDC to Pipeline (3) Fulfills a buy order on OpenSea with the NFT (to receive more USDC) (4) Trades USDC on Uniswap to WETH (5) Sends back WETH to themselves.
2. The malicious buyer reenters Pipeline in step (3) and steals the user's USDC

Step (5) is the so-called unloading step, but the question is: How, specifically, should the user unload to protect themselves from this reentrancy attack?

Method 1: If based on the return value from a swap or balanceOf(), users cannot even notice the attack. Normally, say the user can swap 15000 USDC for 10 WETH (the return value is 10 * 10^18). But, under an attack, suppose the attack transfers out 7500 USDC in step (3), the return value becomes 5 * 10^18. If the user blindly chooses the return value of swap or balance to transfer assets out from Pipeline, they, as a result, receive less amount than they should.

Method 2: Transfer out an exact amount of assets to themselves, e.g., call WETH.transfer(user, 10 * 10^18). This is unfortunately not practical since users need to know the exact amount to transfer before the transaction executes, i.e., when submitting the transaction. The price of WETH to USDC can change during the transaction submission time and execution time. This issue happens in the most typical use cases of swaps (or possibly others such as yield aggregators), regardless of whether the user is under attack.

Suppose the user chooses the value of 10 * 10^18. If the price of WETH increases (even slightly), the return value will become less than 10 * 10^18, and the transaction reverts. The user does not lose value, but the possibility of a transaction reverts is quite often, which causes a waste of the user's gas fees and time. This is why, if not all, AMMs and DEXs do not let you specify an exact return value to receive.

If the price of WETH decreases, the return value will become more than 10 * 10**18. The overall transaction to Pipeline succeeds, but there will be assets left in the Pipeline, which can be dust or possibly a significant amount (if the trade is large or the price changes too quickly). As a result, the user also loses their assets since anyone can transfer assets left in the Pipeline.

I think this reentrancy issue is valid, and the two defense methods you mentioned are either ineffective or not practical (even in normal use cases).