Lack checks if Pool deployed by the canonical UniswapV3Factory in uniswapV3SwapCallback

[code423n4]

Lines of code

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

Vulnerability details

Impact

Loss of deserved tokens or DoS when users use this contract.

Proof of Concept

According to the uniswap docs:
https://docs.uniswap.org/contracts/v3/reference/core/interfaces/callback/IUniswapV3SwapCallback#uniswapv3swapcallback

The caller of this method must be checked to be a UniswapV3Pool deployed by the canonical UniswapV3Factory.

However, there is no check in this contract's implementation. So a malicious user (since anyone can deploy this contract and trick other users into using it) or malicious project owner (not centralization risk I think: code-423n4/2022-10-juicebox-findings#191 (comment)) can create a fake pool, which calls uniswapV3SwapCallback. There are two attacks:

1.Reach the max allowed slippage and get extra tokens. The attacker can do the exchange himself at a low slippage but give back the user with a high allowed slippage.

2.Bypass the slippage but revert later. The attacker can pass nice parameters to uniswapv3swapcallback but doesn't transfer tokens (or few), so he can pass the slippage checks. But the transaction will later revert (DoS) either at:

        // Send the non-reserved token to the beneficiary (if any / reserved rate is not max)
        if (_nonReservedToken != 0) projectToken.transfer(_data.beneficiary, _nonReservedToken);

or

            // 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, // @audit-info is ok for the fake pool attacks above when _reservedToken is low  https://github.com/jbx-protocol/juice-contracts-v3/blob/12d852f28d372dd44987586f8009c56b0fe247a9/contracts/JBTokenStore.sol#L327
                _memo: "",
                _preferClaimedTokens: true
            });

Tools Used

VSCode

Recommended Mitigation Steps

We should use functions like UniswapV3Factory.getPool() to make sure it's a real pool.

Assessed type

Uniswap

[dmvt]

spam / self dos

[c4-pre-sort]

dmvt marked the issue as low quality report

[c4-judge]

dmvt marked the issue as unsatisfactory:
Invalid