Vader Protocol contest - hyh's results

Handle

hyh

Vulnerability details

Impact

Pool funds will be siphoned out over time as swaps and asymmetric LP provision are generally balancing each other economically. While with introduction of IL reimbursement a malicious user can make an asymmetric LP, then profit immediately from out of balance pool with a swap and profit again after a while from IL coverage. This requires locking liquidity to a pool to become covered by IL protection, but still represents an additional profit without additional risk at expense of reserve funds.

Another variant of exploiting this is to add liquidity in two steps: deposit #1 with zero slip adjustment, perfectly matching current market price, deposit #2 with more Vader than market price suggests, moving pool out of balance with Vader becoming cheaper, then exiting deposit #1 with profit because slip adjustment reduced deposit #2 related share issuance and deposit #1 now has more asset claims than before. Deposit #2 then need to wait and exit after some time to receive IL protection coverage: IL is calculated as ((originalAsset * releasedVader) / releasedAsset) + originalVader - ((releasedAsset * releasedVader) / releasedAsset) + releasedVader , i.e. original deposit values without taking account of slip adjustment are used, so providing more Vader in deposit #2 leads to greater IL, which this way have two parts: market movements related and skewed liquidity provision related. IL covering compensates for slip adjustments this way, which is exploitable at scale as long as asymmetric IL is allowed.

Proof of Concept

The steps to reproduce are:

1. Add asymmetric LP via mint (with NFT, for example),
2. Either swap gathering profit from pool skew or do symmetric deposit beforehand and exit it now,
3. Wait for some period for IL covering, and then withdraw, having IL covered by reserve fund.

BasePoolV2 NFT mint: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/BasePoolV2.sol#L479

VaderPoolV2 NFT burn: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L265 IL coverage there: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L293

IL calculation: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex/math/VaderMath.sol#L72

Recommended Mitigation Steps

Our view is that asymmetric liquidity provision doesn't provide much business value, introducing substantial attack surface, so the core recommendation here is to remove a possibility to add liquidity asymmetrically.

For example, the following can be implemented: instead of penalizing LP with slip adjustment do the biggest liquidity additions with zero slip adjustment that user provided funds allow, and then transfer the remaining part of the funds to a user.

Allowing only symmetric liquidity addition completely removes the described attack surface.

Handle

hyh

Vulnerability details

Impact

Users that mint synths do provide native assets, increasing native reserve pool, but do not get any liquidity shares issued. In the same time, an exit of non-synth liquidity provider yields releasing a proportion of all current reserves to him.

Whenever an exit of non-synth LP is substantial enough, the system will have much less native asset regarding the cumulative deposit of synth holders. That is, when a LP entered he provided a share of current reserves, both native and foreign, and got the corresponding liquidity shares in return. Suppose then big enough amounts of synths were minted, providing correspondingly big enough amount of native assets. If the LP now wants to exit, he will obtain a part of total native assets, including a part of the amount that was provided by synth minter. If the exit is big enough there will be substantially less native assets left to reimburse the synth minter than he initially provided. This is not reversible: the synth minters lost their native assets to LP that exited.

Proof of Concept

There are three types of mint/burn: NFT, fungible and synths. First two get LP shares, the latter gets synths. Whenever NFT or fungible LP exits, it gets a proportion of combined reserves. That is, some of native reserves were deposited by synth minters, but it is not accounted anyhow, only one total reserve number per asset is used.

Suppose the following scenario, Alice wants to provide liquidity, while Bob wants to mint synths:

1. Alice deposits both sides to a pool, 100 USDV and 100 foreign.
2. Bob deposit 100 USDV and mints some foreign Synth.
3. LP withdraws 95% of her liquidity shares. As she owns the pool liquidity, she gets 95% of USDV and foreign total reserves, 190 USDV and 95 foreign. Alice received almost all of her and Bob's USDV.
4. If Bob burn his synth and withdraw, he will get a tiny fraction of USDV he deposited (calculated by VaderMath.calculateSwap, we use its terms): https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex/math/VaderMath.sol#L98 x = 100, X = 0.05 * 200 = 10, Y = 0.05 * 100 = 5. Swap outcome, how much USDV will Bob get, is x * Y * X / (x + X) ^ 2 = 100 * 5 * 10 / (110^2) = 0.4 (rounded).

The issue is that synth provided and LP provided USDV aren't accounted separately, total reserves number if used everywhere instead:

Synth minters provide native asset, say USDV, to the system: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L187

Synth minters get synths and no LP shares, while to account for their deposit, the total USDV balance is increased: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L187

When LP enters, it gets liquidity shares proportionally to current reserves (NFT mint, notice the reserveNative, which is BasePoolV2's pair.reserveNative, total amount of native asset in the Pool): https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/BasePoolV2.sol#L497

When LP exits, it gets a proportion of current reserves back (NFT burn): https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/BasePoolV2.sol#L223

The same happens when fungible LP mints (same reserveNative): https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L336 And burns: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/dex-v2/pool/VaderPoolV2.sol#L401

Recommended Mitigation Steps

Account for LP provided liquidity separately from total amount variables, i.e. use only LP provided native reserves variables in LP shares mint and burn calculations. That should suffice as total amount of native assets is still to be used elsewhere, whenever the whole pool is concerned, for example, in rescue function, swap calculations and so forth.