Block reorg can cause unexpected behaviour during deployment of WildcatMarkets which could lead to broken functionality of the protocol

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L319 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L327 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L405 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L428-L423 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L440 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/FixedTermLoanHooks.sol#L133

Vulnerability details

Proof of Concept

The Wildcat protocol is expected to be deployed on Layer 2 block chains such as Arbitrum and Polygon. In these layer2 blockchains the block reorgs are common and can happen frequently (Even in the layer 1 Ethereum also the block reorgs can happen). The following issue can occur if a block reorg happens in any of the above layer 2 blockchains on which the wildcat protocol is deployed on.

Let's consider the following scenario:

1. The HooksFactory.deployHooksInstance function is called by User A to deploy a new hook instance.
2. Then this hook instance is used in the HooksFactory.deployMarket call to deploy a new market by User A.
3. The HooksFactory.deployMarket function calls the _deployMarket function where the msg.sender will be used as the borrower of the WildcatMarket. And the borrower initialized in the hook instance should also belong to the same borrower.
4. Further to that the hooks template corresponding to the hook instance is also set when the hook instance was deployed. And this hook template is used in the _marketsByHooksTemplate mapping to push the newly deployed market into the mapping.
5. But the issue here is that the HooksFactory._deployHooksInstance function uses the create opcode to deploy the hook instance which is susceptible to block reorgs (since it uses the initcode and the nonce of the deployer to determine the deployment address).
6. If a block reorg happens now a different deploy hooks instance could get deployed by User B at the same address as the previous hook instance deployed by User A. This will have a different hook template as well.
7. Now again the HooksFactory.deployMarket function will execute and this will use the hook instance deployed by User B and the market deployer is User A. Hence the borrower of the hooks instance is User B where as the borrower of the market is User A. This will break the onlyBorrower functionality of the hooks instance since User A is not the borrower of the hooks instance.
8. Furthermore due to the above block reorg the hook template used to deploy the market could also change. This will prompt the User A to deploy a market with an unexpected hook template which will break the functionality of his deployed market.
9. Hence it is clear above block reorg can break the intended functionality of thewildcat protocol`.

https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L319 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L327 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L405 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L428-L423 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/HooksFactory.sol#L440 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/access/FixedTermLoanHooks.sol#L133

Recommended Mitigation Steps

Hence it is recommended to use the CREATE2 opcode to deploy the hook instance instead of the CREATE opcode. The CREATE2 opcode can use a salt value which consists of the msg.sender such that the different hook instance addresses will be computed for different deployers of the hook instances.

Assessed type

Other

[c4-judge]

3docSec changed the severity to QA (Quality Assurance)

[c4-judge]

3docSec marked the issue as grade-b

[udsene]

Hi @3docSec,

Thank You for judging this contest.

This issue has been marked as a duplicate of #36. But the issue #36 discusses of the reorg vulnerability present in the HooksFactory.deployMarketAndHooks function. But as you have mentioned in #36 the reorg vulnerability is not present (or at least the impact is very less) in the HooksFactory.deployMarketAndHooks since this function is atomic.

But there is another way where markets can be deployed. That is by calling the HooksFactory.deployHooksInstance and HooksFactory.deployMarket functions separately. Here the HooksFactory.deployMarket function uses an already deployed hooksInstance and the transaction is not atomic. Hence if a reorg happens and the given hooksInstance deployment is also included in a reorg block then the market can be deployed with a different hooksInstance (Since a different hooksInstance is deployed at the same address used for market deployment due to use of create) than expected as explained in this issue.

As a result the market will be deployed with an unexpected hook template due to reorg thus breaking the expected behavior of the market as explained in the original issue.

Let's consider the following scenario:

A user deploys HooksInstance A and HooksInstance B by calling the HooksFactory.deployHooksInstance. He expects to deploy three markets, the Market A with HooksInstance A and Market B with HooksInstance B and Market C with HooksInstance B and triggers three different HooksFactory.deployMarket transactions by passing in the respective HooksInstances. Now a block reorg happens and the Market A gets deployed with HooksInstance B and Market B and Market C gets deployed with HooksInstance A contrary to the expectation of the user. As a result this breaks the intended behavior of the markets by the user (deployer).

The sponsor has acknowledged this issue and has already fixed it here.

As per the C4 Severity Categorization below:

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

I think this falls under Medium severity since the the function of the protocol or its availability could be impacted in this scenario.

Hence can you please dedupe this issue from #36 and reconsider the Medium severity for this issue.

Thank You

[3docSec]

Interesting point @udsene.

The 2-step deployment flow you are highlighting is indeed vulnerable. However, with the reorg scenario we still are in very unlikely territory; the fact the user calls deployHooksInstance and immediately afterwards deployMarket instead of deployMarketAndHooks that is there, available to achieve both, makes it even more unlikely.

I admit there is more subjectiveness in the mix than we'd all like to, but I don't see the scenario likelihood close enough to what an even low-likelihood M would be