Closed code423n4 closed 2 years ago
Disagree with the severity since we are discussing dust amounts. 1 token for a 6 decimal USDC token would be .000001 and worse for an 18 decimal token.
Based on other warden's suggestions we will enforce a minimum amount of liquidity remain locked in the contract that will resolve this dust attack.
Duplicate of #145
I feel like the warden really risked by showing the 1 token (dust) example, instead of showing how this can be done to sandwich depositors and take their underlying.
That said, the attack is same as #145 so the finding get's the same severity
Handle
camden
Vulnerability details
Impact
Read the attack composition below. But the main criteria is that the attacker has to be the first person to provide liquidity. They can (at least from my testing) permanently grief a pool and make it impossible for any later person to get liquidity tokens, only by spending a very minimal amount of tokens.
Proof of Concept
Here's my proof of concept code. You should be able to make a new file in the js test repository and test it with
npx hardhat test test/attackerTest.js
https://gist.github.com/CamdenClark/f66f492d0d5211ecc36c7c808562cfed
The attack is as follows:
Additionally, it appears that the pool is permanently griefed. I haven't been able to write a test where any subsequent liquidity provider is able to get liquidity tokens after reaching this state.
Note that manually transferring the base token to the exchange is mandatory to replicate, if the attacker only initially provides 1 quote and 1 base token, there is no issue.
Tools Used
hardhat
Recommended Mitigation Steps
I haven't stepped through the math enough to understand exactly where the issue is here. I imagine there is some issue with the liquidity token math if you start with only one liquidity token and then have to do a single asset provide liquidity call. Simplifying the math with all the WAD's would make this a lot easier to debug.
I ultimately think that the initial state of the pool is pretty important here, so I think it might be worth making it so only governance is allowed to create pairs + provide initial liquidity so there's enough not to break math precision. Either way, I think that manually sending either base or quote tokens needs to be part of your threat model for the math here, because it seems possible there are other holes.