Sushi Trident contest phase 1 - GreyArt's results

Handle

GreyArt

Vulnerability details

Impact

A number of functions suffer from the erroneous conversion of Balancer V1's implementation.

• _compute() (equivalent to Balancer's [bpow()](https://github.com/balancer-labs/balancer-core/blob/master/contracts/BNum.sol#L108-L126))
  • if (remain == 0) output = wholePow; when a return statement should be used instead.
• _computeSingleOutGivenPoolIn() (equivalent to Balancer's [_calcSingleOutGivenPoolIn()](https://github.com/balancer-labs/balancer-core/blob/master/contracts/BMath.sol#L195-L224))
  • tokenOutRatio should be calculated with _compute() instead of _pow()
  • zaz should be calculated with _mul() instead of the native *
• _pow() (equivalent to Balancer's [bpowi()](https://github.com/balancer-labs/balancer-core/blob/master/contracts/BNum.sol#L89-L103))
  • Missing brackets {} for the for loop causes a different interpretation
  • _mul should be used instead of the native *

Recommended Mitigation Steps

The fixed implementation is provided below.

function _computeSingleOutGivenPoolIn(
  uint256 tokenOutBalance,
  uint256 tokenOutWeight,
  uint256 _totalSupply,
  uint256 _totalWeight,
  uint256 toBurn,
  uint256 _swapFee
) internal pure returns (uint256 amountOut) {
    uint256 normalizedWeight = _div(tokenOutWeight, _totalWeight);
    uint256 newPoolSupply = _totalSupply - toBurn;
    uint256 poolRatio = _div(newPoolSupply, _totalSupply);
    uint256 tokenOutRatio = _compute(poolRatio, _div(BASE, normalizedWeight));
    uint256 newBalanceOut = _mul(tokenOutRatio, tokenOutBalance);
    uint256 tokenAmountOutBeforeSwapFee = tokenOutBalance - newBalanceOut;
    uint256 zaz = _mul(BASE - normalizedWeight, _swapFee);
    amountOut = _mul(tokenAmountOutBeforeSwapFee, (BASE - zaz));
}

function _compute(uint256 base, uint256 exp) internal pure returns (uint256 output) {
  require(MIN_POW_BASE <= base && base <= MAX_POW_BASE, "INVALID_BASE");
  
  uint256 whole = (exp / BASE) * BASE;   
  uint256 remain = exp - whole;
  uint256 wholePow = _pow(base, whole / BASE);
  
  if (remain == 0) return wholePow;
  
  uint256 partialResult = _powApprox(base, remain, POW_PRECISION);
  output = _mul(wholePow, partialResult);
}

function _pow(uint256 a, uint256 n) internal pure returns (uint256 output) {
  output = n % 2 != 0 ? a : BASE;
  for (n /= 2; n != 0; n /= 2) {
		a = _mul(a, a);
    if (n % 2 != 0) output = _mul(output, a);
	}
}

Handle

GreyArt

Vulnerability details

Impact

A mint fee is applied whenever unbalanced liquidity is added, because it is akin to swapping the excess token amount for the other token.

However, the current implementation distributes the minted fee to the minter as well (when he should be excluded). It therefore acts as a rebate of sorts.

As a result, it makes adding and removing liquidity as opposed to swapping directly (negligibly) more desirable. An example is given below using the Constant Product Pool to illustrate this point. The Hybrid pool exhibits similar behaviour.

Proof of Concept

1. Initialize the pool with ETH-USDC sushi pool amounts. As of the time of writing, there is roughly 53586.556 ETH and 165143020.5295 USDC.
2. Mint unbalanced LP with 5 ETH (& 0 USDC). This gives the user 138573488720892 / 1e18 LP tokens.
3. Burn the minted LP tokens, giving the user 2.4963 ETH and 7692.40 USDC. This is therefore equivalent to swapping 5 - 2.4963 = 2.5037 ETH for 7692.4044 USDC.
4. If the user were to swap the 2.5037 ETH directly, he would receive 7692.369221 (0.03 USDC lesser).

Recommended Mitigation Steps

The mint fee should be distributed to existing LPs first, by incrementing _reserve0 and _reserve1 with the fee amounts. The rest of the calculations follow after.

ConstantProductPool

(uint256 fee0, uint256 fee1) = _nonOptimalMintFee(amount0, amount1, _reserve0, _reserve1);
// increment reserve amounts with fees
_reserve0 += uint112(fee0);
_reserve1 += uint112(fee1);
unchecked {
    _totalSupply += _mintFee(_reserve0, _reserve1, _totalSupply);
}
uint256 computed = TridentMath.sqrt(balance0 * balance1);
...
kLast = computed;

HybridPool

(uint256 fee0, uint256 fee1) = _nonOptimalMintFee(amount0, amount1, _reserve0, _reserve1);
// increment reserve amounts with fees
_reserve0 += uint112(fee0);
_reserve1 += uint112(fee1);
uint256 newLiq = _computeLiquidity(balance0, balance1);
...