Function didPay() Malfunction

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L302-L309 https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L315-L322

Vulnerability details

Impact

The didPay() function essentially handles the payment process and determines whether to swap tokens or mint new tokens based on the provided data and preferences.Inside the function, if _data.preferClaimedTokens is true, it attempts to swap tokens using the _swap function, passing _data, _minimumReceivedFromSwap, and _reservedRate as arguments.

  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);
        }

Inside the internal _swap() function, the protocol performs a token swap on the Uniswap V3 platform. As the code below If there are reserved tokens (when _reservedToken is not zero), the function performs the following steps: a) It retrieves the controller contract associated with the project using the jbxTerminal.directory().controllerOf function. b) It burns all the reserved tokens held by the current contract address. c) It mints the reserved tokens, with the current contract address as the beneficiary, using the controller.mintTokensOf function. d) It burns any remaining non-reserved tokens held by the current contract address (if any). However,the burnTokensOf() and mintTokensOf() calling code violates the interface requirements by using struct-like syntax to pass the parameters.

 if (_reservedToken != 0) {
            IJBController controller = IJBController(jbxTerminal.directory().controllerOf(_data.projectId));

            // 1) Burn all the reserved token, which are in this address -> result: 0 here, 0 in reserve
            controller.burnTokensOf({
                _holder: address(this),
                _projectId: _data.projectId,
                _tokenCount: _reservedToken,
                _memo: "",
                _preferClaimedTokens: true
            });

            // 2) Mint the reserved token with this address as beneficiary -> result: _amountReceived-reserved here, reservedToken in reserve
            controller.mintTokensOf({
                _projectId: _data.projectId,
                _tokenCount: _amountReceived,
                _beneficiary: address(this),
                _memo: _data.memo,
                _preferClaimedTokens: false,
                _useReservedRate: true
            });

According to the interface definition, the functions mintTokensOf() and burnTokensOf() require parameters in the following format.

  function mintTokensOf(
    uint256 _projectId,
    uint256 _tokenCount,
    address _beneficiary,
    string calldata _memo,
    bool _preferClaimedTokens,
    bool _useReservedRate
  ) external returns (uint256 beneficiaryTokenCount);

  function burnTokensOf(
    address _holder,
    uint256 _projectId,
    uint256 _tokenCount,
    string calldata _memo,
    bool _preferClaimedTokens
  ) external;

This could potentially result in the malfunctioning of the functions.

Proof of Concept

1.The functions mintTokensOf and burnTokensOf require parameters of the following types: https://github.com/jbx-protocol/juice-contracts-v3/blob/main/contracts/interfaces/IJBController3_1.sol#L146C1-L161

  function mintTokensOf(
    uint256 _projectId,
    uint256 _tokenCount,
    address _beneficiary,
    string calldata _memo,
    bool _preferClaimedTokens,
    bool _useReservedRate
  ) external returns (uint256 beneficiaryTokenCount);

2.Passing a struct type parameter in the swap function may cause the function to malfunction.

  controller.mintTokensOf({
                _projectId: _data.projectId,
                _tokenCount: _amountReceived,
                _beneficiary: address(this),
                _memo: _data.memo,
                _preferClaimedTokens: false,
                _useReservedRate: true
            });

Tools Used

Vscode

Recommended Mitigation Steps

The correct way to call the function is to pass the parameter values in the order they are defined, as shown below: controller.mintTokensOf( _data.projectId, _amountReceived, address(this), _data.memo, false, true );

Assessed type

Other

[c4-pre-sort]

dmvt marked the issue as low quality report

[dmvt]

No specific impact or example described.

[c4-judge]

dmvt marked the issue as unsatisfactory: Insufficient quality