QuickSwap and StellaSwap contest - neko_nyaa's results

Lines of code

https://github.com/code-423n4/2022-09-quickswap/blob/main/src/core/contracts/AlgebraPool.sol#L193-L206

Vulnerability details

Proof of Concept

There is no access control on AlgebraPool.initialize(). That means anyone can set an initial pool price, which can be any value.

This creates an arbitrage opportunity. An exploit scenario is as follow:

• Alice deploys a pool for two tokens A and B. Let's assume the market price at time of deployment is 1 A = 1 B.
• Bob manages to call initialize() with a bad price, say 1 A = 10 B.
• Alice adds liquidity to the pool by calling mint(). Note that Alice has to add liquidity at Bob's ratio.

The result is that Alice's added liquidity ratio is radically different from the market, exposing her position to massive impermanent loss, and Bob (as well as others) can arbitrage off this pool.

Tools Used

VSCode

Recommended Mitigation Steps

The key point here is that the pool deployer's operations should be atomic. That is, no one should be able to perform any pool operations before settings are complete.

The recommended solution is to add proper access controls to initialize(), ensuring only the pool deployer can set the initial price. However, this should potentially come with a time release, to allow anyone to set initial price if the deployer fails to initialize within a reasonable time frame (preventing a DoS).

Adding initial price as a constructor parameter is not a solution, as it can change the contract creation code, invalidating the assumption that we can pre-calculate pool addresses before deployment.