Amount of project token minted to beneficiary by JBXBuybackDelegate._mint function is not checked against an expected minimum number of project tokens to be minted to such beneficiary

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L144-L171
https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L183-L209
https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L334-L353

Vulnerability details

Impact

Calling the following JBPayoutRedemptionPaymentTerminal3_1._pay function executes (_fundingCycle, _tokenCount, _delegateAllocations, _memo) = store.recordPaymentFrom(_payer, _bundledAmount, _projectId, baseWeightCurrency, _beneficiary, _memo, _metadata).

File: node_modules\@jbx-protocol\juice-contracts-v3\contracts\abstract\JBPayoutRedemptionPaymentTerminal3_1.sol
1444:   function _pay(
1445:     uint256 _amount,
1446:     address _payer,
1447:     uint256 _projectId,
1448:     address _beneficiary,
1449:     uint256 _minReturnedTokens,
1450:     bool _preferClaimedTokens,
1451:     string memory _memo,
1452:     bytes memory _metadata
1453:   ) internal returns (uint256 beneficiaryTokenCount) {
...
1469:       // Record the payment.
1470:       (_fundingCycle, _tokenCount, _delegateAllocations, _memo) = store.recordPaymentFrom(
1471:         _payer,
1472:         _bundledAmount,
1473:         _projectId,
1474:         baseWeightCurrency,
1475:         _beneficiary,
1476:         _memo,
1477:         _metadata
1478:       );
1479: 
1480:       // Mint the tokens if needed.
1481:       if (_tokenCount > 0)
1482:         // Set token count to be the number of tokens minted for the beneficiary instead of the total amount.
1483:         beneficiaryTokenCount = IJBController(directory.controllerOf(_projectId)).mintTokensOf(
1484:           _projectId,
1485:           _tokenCount,
1486:           _beneficiary,
1487:           '',
1488:           _preferClaimedTokens,
1489:           true
1490:         );
1491: 
1492:       // The token count for the beneficiary must be greater than or equal to the minimum expected.
1493:       if (beneficiaryTokenCount < _minReturnedTokens) revert INADEQUATE_TOKEN_COUNT();
1494: 
1495:       // If delegate allocations were specified by the data source, fulfill them.
1496:       if (_delegateAllocations.length != 0) {
...
1513:         // Get a reference to the number of delegates to allocate to.
1514:         uint256 _numDelegates = _delegateAllocations.length;
1515: 
1516:         for (uint256 _i; _i < _numDelegates; ) {
...
1523:           // Keep track of the msg.value to use in the delegate call
1524:           uint256 _payableValue;
1525: 
1526:           // If this terminal's token is ETH, send it in msg.value.
1527:           if (token == JBTokens.ETH) _payableValue = _delegateAllocation.amount;
...
1532:           _delegateAllocation.delegate.didPay{value: _payableValue}(_data);
...
1541:           unchecked {
1542:             ++_i;
1543:           }
1544:         }
1545:       }
1546:     }
...
1560:   }

Then, calling the following JBSingleTokenPaymentTerminalStore3_1.recordPaymentFrom function executes (_weight, memo, delegateAllocations) = IJBFundingCycleDataSource(fundingCycle.dataSource()).payParams(_data).

File: node_modules\@jbx-protocol\juice-contracts-v3\contracts\JBSingleTokenPaymentTerminalStore3_1.sol
323:   function recordPaymentFrom(
324:     address _payer,
325:     JBTokenAmount calldata _amount,
326:     uint256 _projectId,
327:     uint256 _baseWeightCurrency,
328:     address _beneficiary,
329:     string calldata _memo,
330:     bytes memory _metadata
331:   )
332:     external
333:     override
334:     nonReentrant
335:     returns (
336:       JBFundingCycle memory fundingCycle,
337:       uint256 tokenCount,
338:       JBPayDelegateAllocation[] memory delegateAllocations,
339:       string memory memo
340:     )
341:   {
...
351:     // The weight according to which new token supply is to be minted, as a fixed point number with 18 decimals.
352:     uint256 _weight;
353: 
354:     // If the funding cycle has configured a data source, use it to derive a weight and memo.
355:     if (fundingCycle.useDataSourceForPay() && fundingCycle.dataSource() != address(0)) {
356:       // Create the params that'll be sent to the data source.
357:       JBPayParamsData memory _data = JBPayParamsData(
358:         IJBSingleTokenPaymentTerminal(msg.sender),
359:         _payer,
360:         _amount,
361:         _projectId,
362:         fundingCycle.configuration,
363:         _beneficiary,
364:         fundingCycle.weight,
365:         fundingCycle.reservedRate(),
366:         _memo,
367:         _metadata
368:       );
369:       (_weight, memo, delegateAllocations) = IJBFundingCycleDataSource(fundingCycle.dataSource())
370:         .payParams(_data);
371:     }
372:     // Otherwise use the funding cycle's weight
373:     else {
...
376:     }
...
414:     // If there's no weight, token count must be 0 so there's nothing left to do.
415:     if (_weight == 0) return (fundingCycle, 0, delegateAllocations, memo);
...
428:   }

If _tokenCount < _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR) is true, calling the following JBXBuybackDelegate.payParams function would return a 0 weight. When _weight == 0 is true in the JBSingleTokenPaymentTerminalStore3_1.recordPaymentFrom function, it would return a 0 tokenCount. Back in the JBPayoutRedemptionPaymentTerminal3_1._pay function, when _tokenCount is 0, beneficiaryTokenCount would remain 0 that is its default value, and executing if (beneficiaryTokenCount < _minReturnedTokens) revert INADEQUATE_TOKEN_COUNT() can revert when the _minReturnedTokens input is positive. Thus, in order ot make such transaction to go through, the user who calls the JBPayoutRedemptionPaymentTerminal3_1.pay function that further calls the JBPayoutRedemptionPaymentTerminal3_1._pay function must input _minReturnedTokens as 0 in this situation.

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L144-L171

    function payParams(JBPayParamsData calldata _data)
        external
        override
        returns (uint256 weight, string memory memo, JBPayDelegateAllocation[] memory delegateAllocations)
    {
        // Find the total number of tokens to mint, as a fixed point number with 18 decimals
        uint256 _tokenCount = PRBMath.mulDiv(_data.amount.value, _data.weight, 10 ** 18);

        // Unpack the quote from the pool, given by the frontend
        (,, uint256 _quote, uint256 _slippage) = abi.decode(_data.metadata, (bytes32, bytes32, uint256, uint256));

        // If the amount swapped is bigger than the lowest received when minting, use the swap pathway
        if (_tokenCount < _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR)) {
            // Pass the quote and reserve rate via a mutex
            mintedAmount = _tokenCount;
            reservedRate = _data.reservedRate;

            // Return this delegate as the one to use, and do not mint from the terminal
            delegateAllocations = new JBPayDelegateAllocation[](1);
            delegateAllocations[0] =
                JBPayDelegateAllocation({delegate: IJBPayDelegate(this), amount: _data.amount.value});

            return (0, _data.memo, delegateAllocations);
        }

        ...
    }

Furthermore, when the JBPayoutRedemptionPaymentTerminal3_1._pay function executes _delegateAllocation.delegate.didPay{value: _payableValue}(_data), the following JBXBuybackDelegate.didPay function can execute _mint(_data, _tokenCount) if the swap fails or _data.preferClaimedTokens is false. However, when calling the JBXBuybackDelegate._mint function below, the return value of the controller.mintTokensOf function call is not used to check against a value that is similar to _minReturnedTokens, which is unlike if (beneficiaryTokenCount < _minReturnedTokens) revert INADEQUATE_TOKEN_COUNT(), where beneficiaryTokenCount is the return value of the IJBController(directory.controllerOf(_projectId)).mintTokensOf function call, in the JBPayoutRedemptionPaymentTerminal3_1._pay function. As a result, the amount of project token minted to the corresponding beneficiary is not checked against an expected minimum number of project tokens to be minted to such beneficiary. Because of such lack of slippage control, the actual amount of project token minted to the beneficiary can be lower than expected, such as when reservedRate is unexpectedly changed for the current funding cycle.

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L183-L209

    function didPay(JBDidPayData calldata _data) external payable override {
        ...

        // Retrieve the number of token created if minting and reset the mutex (not exposed in JBDidPayData)
        uint256 _tokenCount = mintedAmount;
        mintedAmount = 1;

        // Retrieve the fc reserved rate and reset the mutex
        uint256 _reservedRate = reservedRate;
        reservedRate = 1;

        // The minimum amount of token received if swapping
        (,, uint256 _quote, uint256 _slippage) = abi.decode(_data.metadata, (bytes32, bytes32, uint256, uint256));
        uint256 _minimumReceivedFromSwap = _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR);

        // Pick the appropriate pathway (swap vs mint), use mint if non-claimed prefered
        if (_data.preferClaimedTokens) {
            // Try swapping
            uint256 _amountReceived = _swap(_data, _minimumReceivedFromSwap, _reservedRate);

            // If swap failed, mint instead, with the original weight + add to balance the token in
            if (_amountReceived == 0) _mint(_data, _tokenCount);
        } else {
            _mint(_data, _tokenCount);
        }
    }

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L334-L353

    function _mint(JBDidPayData calldata _data, uint256 _amount) internal {
        IJBController controller = IJBController(jbxTerminal.directory().controllerOf(_data.projectId));

        // Mint to the beneficiary with the fc reserve rate
        controller.mintTokensOf({
            _projectId: _data.projectId,
            _tokenCount: _amount,
            _beneficiary: _data.beneficiary,
            _memo: _data.memo,
            _preferClaimedTokens: _data.preferClaimedTokens,
            _useReservedRate: true
        });

        ...
    }

Proof of Concept

The following steps can occur for the described scenario.

1. Alice calls the JBPayoutRedemptionPaymentTerminal3_1.pay function, which further calls the JBPayoutRedemptionPaymentTerminal3_1._pay function, to pay some ETH.
2. Because _tokenCount < _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR) in the JBXBuybackDelegate.payParams function would be true for her transaction, the swap pathway could be used.
3. In order to make her transaction go through, Alice's transaction uses 0 as the _minReturnedTokens input for the JBPayoutRedemptionPaymentTerminal3_1.pay function call.
4. Yet, the swap fails for some reason so the JBXBuybackDelegate._mint function is called.
5. Since the _minReturnedTokens input is 0 when calling the JBPayoutRedemptionPaymentTerminal3_1.pay function and the amount of project token minted to her beneficiary by the JBXBuybackDelegate._mint function is not checked against an expected minimum number of project tokens to be minted to such beneficiary, an unexpected change of reservedRate for the current funding cycle can cause the actual amount of project token minted to her beneficiary to be lower than expected.

Tools Used

VSCode

Recommended Mitigation Steps

The JBPayoutRedemptionPaymentTerminal3_1._pay function can be updated to execute if (beneficiaryTokenCount < _minReturnedTokens) revert INADEQUATE_TOKEN_COUNT() only when _tokenCount > 0 is true and pass _minReturnedTokens to the JBXBuybackDelegate.didPay function. Then, the JBXBuybackDelegate.didPay and JBXBuybackDelegate._mint functions can be updated to be able to receive _minReturnedTokens. Moreover, the JBXBuybackDelegate._mint function can be updated to revert if the return value of the controller.mintTokensOf function call is less than _minReturnedTokens.

Assessed type

Invalid Validation

[c4-pre-sort]

dmvt marked the issue as duplicate of #36

[c4-judge]

dmvt marked the issue as satisfactory