Juicebox Buyback Delegate - T1MOH's results

Lines of code

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

Vulnerability details

Impact

didPay() function is payable and receives ETH. Then during uniswapV3Callback deposits ETH into WETH and sends it into UniswapV3Pool for projectToken. But excess ETH is not returned to caller of JBXBuybackDelegate and can be used by other users.

Proof of Concept

didPay() is payable:

    function didPay(JBDidPayData calldata _data) external payable override {

Excess is not returned:

    function uniswapV3SwapCallback(int256 amount0Delta, int256 amount1Delta, bytes calldata data) external override {
        // Check if this is really a callback
        if (msg.sender != address(pool)) revert JuiceBuyback_Unauthorized();

        // Unpack the data
        (uint256 _minimumAmountReceived) = abi.decode(data, (uint256));

        // Assign 0 and 1 accordingly
        uint256 _amountReceived = uint256(-(_projectTokenIsZero ? amount0Delta : amount1Delta));
        uint256 _amountToSend = uint256(_projectTokenIsZero ? amount1Delta : amount0Delta);

        // Revert if slippage is too high
        if (_amountReceived < _minimumAmountReceived) revert JuiceBuyback_MaximumSlippage();

        // Wrap and transfer the weth to the pool
        weth.deposit{value: _amountToSend}();
        weth.transfer(address(pool), _amountToSend);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Add argument excess receiver to data argument passed to pool.swap(). And send excess:

    function uniswapV3SwapCallback(int256 amount0Delta, int256 amount1Delta, bytes calldata data) external override {
        // Unpack the data
        (uint256 _minimumAmountReceived, address excessReceiver) = abi.decode(data, (uint256, address));
        ...

        // Wrap and transfer the weth to the pool
        weth.deposit{value: _amountToSend}();
        weth.transfer(address(pool), _amountToSend);

        // SOLUTION
        payable(excessReceiver).call{value: address(this).balance}("");
    }

It may require to refactor logic of terminal contract interacting with JBXBuybackDelegate

Assessed type

ETH-Transfer