Juicebox Buyback Delegate - ktg's results

Lines of code

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

Vulnerability details

Impact

• If JBXBuybackDelegate is used as data source for redeem, users could not redeem their tokens.

Proof of Concept

The contract JBXBuybackDelegate is a data source and according to the doc: A data source contract can be used to provide custom data to the JBPayoutRedemptionPaymentTerminal3_1.pay(...) transaction and/or the JBPayoutRedemptionPaymentTerminal3_1.redeemTokensOf(...) transaction.

Since JBXBuybackDelegate implements interface IJBFundingCycleDataSource, it must override function redeemParams and the contract decides to leave it empty:

function redeemParams(JBRedeemParamsData calldata _data)
        external
        override
        returns (uint256 reclaimAmount, string memory memo, JBRedemptionDelegateAllocation[] memory delegateAllocations)
    {}

Because the function is empty, if JBXBuybackDelegate is used as datasource for redeem, users would not be able to redeem their tokens, since the returned reclaimAmount from redeemParams is always 0.

Although this contract JBXBuybackDelegate is meant to be used for pay only and not for redeem, the contract should also let function redeemParams return default value (to not interfere with redeem process) so that if a user use JBXBuybackDelegate as datasource for redeem (it's perfectly possible since this contract implements IJBFundingCycleDataSource interface), the contract would not make reclaimedAmount = 0, as recommended in the doc: https://docs.juicebox.money/dev/build/treasury-extensions/data-source/#examples

// This is unused but needs to be included to fulfill IJBFundingCycleDataSource.
  function redeemParams(JBRedeemParamsData calldata _data)
    external
    pure
    override
    returns (
      uint256 reclaimAmount,
      string memory memo,
      IJBRedemptionDelegate delegate
    )
  {
    // Return the default values.
    return (_data.reclaimAmount.value, _data.memo, IJBRedemptionDelegate(address(0)));
  }

Tools Used

Recommended Mitigation Steps

I recommend returning default values from redeemParams function as the code above.

Assessed type

Library