code423n4 commented 1 year ago

Lines of code

https://github.com/jbx-protocol/juice-nft-rewards/blob/f9893b1497098241dd3a664956d8016ff0d0efd0/contracts/JBTiered721DelegateStore.sol#L565

Vulnerability details

Description

This is another bug in the redemption weight calculation mechanism discussed in an issue with the same title. I recommend to read that first for context.

Let's look at DelegateStore's totalRedemptionWeight implementation again:

for (uint256 _i; _i < _maxTierId; ) {
  // Keep a reference to the stored tier.
  _storedTier = _storedTierOf[_nft][_i + 1];
  // Add the tier's contribution floor multiplied by the quantity minted.
  weight +=
    (_storedTier.contributionFloor *
      (_storedTier.initialQuantity - _storedTier.remainingQuantity)) +
    _numberOfReservedTokensOutstandingFor(_nft, _i, _storedTier);
  unchecked {
    ++_i;
  }
}

We can see that the reservedTokensOutstanding is added to the product of contribution * minted tokens. This is a severe mistake, because the weight is meant to take into account the specific tier of the outstanding tokens. Currently all reserved tokens are added linearly to the result and will be negligible if contributionFloor is much larger than 1.

Note that after the reserved tokens are minted, the reservedTokensOutstanding and remainingQuantity count will be decremented, making the minted tokens multiply by contributionFloor. Therefore, we have conflicting weights while no money has been put in.

The impact of this bug is incorrect calculation of the weight of user's contributions. While reserve tokens are yet to be minted, users can redeem tokens and receive much more than they should have, at the expense of the reserved token beneficiary.

Impact

Project reserve tokens lose value when users redeem their NFTs before reserves were minted.

Proof of Concept

1. Suppose we have a project with 2 tiers, reserve ratio = 50%, redemption ratio = 100%:


Tier	Contribution	Initial quantity	Remaining quantity	Reserves minted	Reserves outstanding
Tier 1	50	10	4	0	3

When calculating totalRedemptionWeight(), the correct result is

50 * (10 - 4 + 3) = 450

The wrong result will be:

50 * (10 - 4) + 3 = 303

Therefore, when users redeem NFT rewards, they will get far more value than they are eligible for. This means eventually when the reserve tokens will be minted, they will be worth much less than they are supposed to.

Tools Used

Manual audit

Recommended Mitigation Steps

Change the calculation to:

_storedTier.contributionFloor *
      (_storedTier.initialQuantity - _storedTier.remainingQuantity + 
      _numberOfReservedTokensOutstandingFor(_nft, _i + 1, _storedTier) )