DOS if user chooses to specify minReturnedTokens in the pay function of the terminal

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L166
https://github.com/jbx-protocol/juice-contracts-v3/blob/main/contracts/JBSingleTokenPaymentTerminalStore3_1.sol#L415
https://github.com/jbx-protocol/juice-contracts-v3/blob/main/contracts/abstract/JBPayoutRedemptionPaymentTerminal3_1.sol#L1493

Vulnerability details

Impact

If user specifies minReturnedTokens in the pay function of the terminal and BuybackDelegate will take the swap path, transaction will be reverted due to the failed condition if (beneficiaryTokenCount < _minReturnedTokens) revert INADEQUATE_TOKEN_COUNT(); in the terminal.

Proof of Concept

In the BuybackDelegate contract if swapping is a preferred method of the acquiring JBX tokens we return with a weight = 0

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

In the payment terminal store we also return early with a tokenCount = 0

https://github.com/jbx-protocol/juice-contracts-v3/blob/main/contracts/JBSingleTokenPaymentTerminalStore3_1.sol#L414C1-L415

And finally in the payment terminal we compare beneficiaryTokenCount < _minReturnedTokens where beneficiaryTokenCount is a variable which is set if tokenCount > 0, in our case tokenCount = 0 therefore beneficiaryTokenCount will also be 0. This will result in failed transaction everytime minReturnedTokens > 0.

Tools Used

Manual review

Recommended Mitigation Steps

Perhaps we can add another condition to the check

if (beneficiaryTokenCount < _minReturnedTokens) && _delegateAllocations.length == 0 revert INADEQUATE_TOKEN_COUNT();

This will revert only if mint path was chosen and we don't use the BuybackDelegate. However there is a possibility that this solution will conflict with data sources other than BuybackDelegate.

Assessed type

DoS

[c4-pre-sort]

dmvt marked the issue as duplicate of #36

[c4-judge]

dmvt marked the issue as satisfactory