QuickSwap and StellaSwap contest - minhtrng's results

Lines of code

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

Vulnerability details

Impact

The initialize-function of an AlgebraPool can be frontrun to set an arbitrary initial price. This will negatively affect the first person to add liquidity to the pool.

Proof of Concept

There are no restrictions on the AlgebraPool.initialize(price) function and it is not called as part of the deployment process.

So a malicious user (call her Eve) could look on the mempool for a sequence of deploy->initialize->mint transactions of someone (call her Alice) who deploys a pool for DAI and her own Token A and wants to add liquidity at the same time (for example by using a deploy script, but would work the same if she just manually sends transactions over multiple blocks). Alice wants an initial ratio of 1:10 (1 DAI can be exchanged for 10 A tokens). Now Eve could frontrun the initialize tx and set the ratio to 1:100 instead, so that Alices mint transaction would cause her to add more A tokens than she intended to add, which Eve could then buy for much less DAI than she would have had to pay for else.

The following small snippet demonstrates the difference between initializing price with a ratio of 1:10 vs 1:100 (code mostly from AlgebraPool.spec.ts, just added the frontrun test case):

  //...

  describe('#mint', () => {
    it('fails if not initialized', async () => {
      await expect(mint(wallet.address, -tickSpacing, tickSpacing, 1)).to.be.revertedWith('LOK')
    })

    describe('frontrun initialization', () => {
        beforeEach('initialize the pool at price of 100:1', async () => {
        await pool.initialize(encodePriceSqrt(1, 100))
        await mint(wallet.address, minTick, maxTick, 3161)
      })

        it('has disadvantageous token ratio', async () => {
          expect(await token0.balanceOf(pool.address)).to.eq(31610)
          expect(await token1.balanceOf(pool.address)).to.eq(317)
        })
    })

    describe('after initialization', () => {
      beforeEach('initialize the pool at price of 10:1', async () => {
        await pool.initialize(encodePriceSqrt(1, 10))
        await mint(wallet.address, minTick, maxTick, 3161)
      })

      describe('success cases', () => {
        it('initial balances', async () => {
          expect(await token0.balanceOf(pool.address)).to.eq(9996)
          expect(await token1.balanceOf(pool.address)).to.eq(1000)
        })
        //...
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Options:

1. Add access control
2. Pass an initial price to the AlgebraFactory which gets passed to the AlgebraPoolDeployer, which then calls the initialize-function with that initial price right after pool-deployment.