sherlock-audit / 2023-07-kyber-swap-judging

12 stars 8 forks source link

0x52 - Repeatedly adding dust liquidity to a position can be used to DOS it #91

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0x52

medium

Repeatedly adding dust liquidity to a position can be used to DOS it

Summary

Adding liquidity to any token is permissionless. Due to a quirk of how lockTime is calculated, adding even a single wei of liquidity will increase lockTime by 1 second. Repeatedly adding liquidity allows lockTime to be pushed 1 second AHEAD of current time which will lead to any further calls temporarily reverting. This can be used to DOS the token, preventing it from adding or removing any liquidity. This tactic can be used to DOS large liquidity providers (such as integrating protocols) during times a high volatility, to materially damage the LP via forced impermanent loss.

Vulnerability Detail

AntiSnipAttack.sol#L120-L128

if (isAddLiquidity) {
  self.lockTime = Math
  .ceilDiv(
    Math.max(_self.lockTime, currentTime - vestingPeriod) *
      uint256(currentLiquidity) +
      uint256(uint128(liquidityDelta)) *
      currentTime,
    updatedLiquidity
  ).toUint32();

Whenever liquidity is added to a position, lockTime is updated using the above math. This utilizes ceilDiv when calculating, which always rounds up. The result is that even adding a single wei of liquidity will cause self.lockTime to increase by 1 second. This can be spammed repeatedly to inch up the lock. Eventually, lockTime can even be increase to be larger than currentTime.

AntiSnipAttack.sol#L72-L75

  uint256 feesClaimableSinceLastActionFeeUnits = Math.min(
    C.FEE_UNITS,
    (uint256(currentTime - _self.lockTime) * C.FEE_UNITS) / vestingPeriod
  );

This causes the following lines to underflow and revert, which DOS's that token, preventing any deposits or withdrawals. As explained above this can material damage the user(s) being attacked.

Impact

Temporary DOS leading to loss to LP

Code Snippet

AntiSnipAttack.sol#L49-L132

Tool used

Manual Review

Recommendation

Use min to prevent self.lockTime from exceeding current time

sherlock-admin commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

Trumpero commented:

invalid. In case isAddliquidity, updatedLiquidity = currentLiquidity + liquidityDelta -> self.lockTime = (max(_self.lockTime, currentTime - vestingPeriod) currentLiquidity + liquidityDelta currentTime) / (currentLiquidity + liquidityDelta) (roundingUp) -> self.lockTime always < currentTime

jkoppel commented:

Really nice find! Are there any chains now or in 10 years where calling addLiquidity once per second is feasible?