code-423n4 / 2022-01-elasticswap-findings

1 stars 0 forks source link

LP inflation attack is possible as pools can be created with zero liquidity #108

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

hyh

Vulnerability details

Impact

A griefing by LP inflation attack is possible: an attacker can create pools for popular token pairs, provide a tiny amount of initial liquidity with addLiquidity(), then send big enough amounts of base and quote tokens to the pool contract (Exchange) just created, making the tiny amount of LP tokens correspond to big enough balances of base and quote, i.e. LP token will become prohibitory costly for anyone else to provide liquidity.

The pool will be inoperable as a result, while no new pools can be created for the same token pairs as ExchangeFactory require them to be unique. This way the attacker can deny the creation of an operable pool for any given pair.

Proof of Concept

createNewExchange require new pool to be unique, but no liquidity has to be added:

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/ExchangeFactory.sol#L50

Initial liquidity is set by the first LP (desired amounts are user provided addLiquidity function arguments):

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/libraries/MathLib.sol#L494-511

All the subsequent LP providers will deal with base and quote pool reserves:

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/libraries/MathLib.sol#L401-493

Whose values can be inflated by sending tokens to the pool contract:

https://github.com/code-423n4/2022-01-elasticswap/blob/main/elasticswap/src/contracts/Exchange.sol#L103-104

This attack vector was discovered for Uniswap V2 (in a bit different setting, here inflating the balances gives the same impact as burn there):

https://docs.vexchange.io/audit.html#orgc7f8ae1

Recommended Mitigation Steps

Consider requiring initial LP provision on pool creation and burning of fixed amount of initial LP tokens

0xean commented 2 years ago

We are planning on implementing a minimum locked token balance similar to Uni v2.

Would say this is a med sev issue as it affects our availability but no assets are at risk

GalloDaSballo commented 2 years ago

Duplicate of #145