Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Low Liquidity in Uniswap V3 Pool Can Lead to ETH Lockup in JBXBuybackDelegate Contract #162

Open
code423n4 opened this issue May 22, 2023 · 5 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue M-02 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

The JBXBuybackDelegate contract employs Uniswap V3 to perform ETH-to-project token swaps. When the terminal invokes the JBXBuybackDelegate.didPay() function, it provides the amount of ETH to be swapped for project tokens. The swap operation sets sqrtPriceLimitX96 to the lowest possible price, and the slippage is checked at the callback.

However, if the Uniswap V3 pool lacks sufficient liquidity or being manipulated before the transaction is executed, the swap will halt once the pool's price reaches the sqrtPriceLimitX96 value. Consequently, not all the ETH sent to the contract will be utilized, resulting in the remaining ETH becoming permanently locked within the contract.

Proof of Concept

The _swap() function interacts with the Uniswap V3 pool. It sets sqrtPriceLimitX96 to the minimum or maximum feasible value to ensure that the swap attempts to utilize all available liquidity in the pool.

try pool.swap({
    recipient: address(this),
    zeroForOne: !_projectTokenIsZero,
    amountSpecified: int256(_data.amount.value),
    sqrtPriceLimitX96: _projectTokenIsZero ? TickMath.MAX_SQRT_RATIO - 1 : TickMath.MIN_SQRT_RATIO + 1,
    data: abi.encode(_minimumReceivedFromSwap)
}) returns (int256 amount0, int256 amount1) {
    // Swap succeeded, take note of the amount of projectToken received (negative as it is an exact input)
    _amountReceived = uint256(-(_projectTokenIsZero ? amount0 : amount1));
} catch {
    // implies _amountReceived = 0 -> will later mint when back in didPay
    return _amountReceived;
}

In the Uniswap V3 pool, this check stops the loop if the price limit is reached or the entire input has been used. If the pool does not have enough liquidity, it will still do the swap until the price reaches the minimum/maximum price.

// continue swapping as long as we haven't used the entire input/output and haven't reached the price limit
while (state.amountSpecifiedRemaining != 0 && state.sqrtPriceX96 != sqrtPriceLimitX96) {
    StepComputations memory step;

    step.sqrtPriceStartX96 = state.sqrtPriceX96;

    (step.tickNext, step.initialized) = tickBitmap.nextInitializedTickWithinOneWord(
        state.tick,
        tickSpacing,
        zeroForOne
    );

Finally, the uniswapV3SwapCallback() function uses the input from the pool callback to wrap ETH and transfer WETH to the pool. So, if _amountToSend < msg.value, the unused ETH is locked in the contract.

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

Consider returning the amount of unused ETH to the beneficiary.

Assessed type

Other

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels May 22, 2023
code423n4 added a commit that referenced this issue May 22, 2023
@c4-pre-sort
Copy link

dmvt marked the issue as duplicate of #42

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jun 2, 2023
@c4-judge
Copy link

c4-judge commented Jun 2, 2023

dmvt changed the severity to 2 (Med Risk)

@c4-judge c4-judge reopened this Jun 2, 2023
@c4-judge c4-judge added primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report labels Jun 2, 2023
@c4-judge
Copy link

c4-judge commented Jun 2, 2023

dmvt marked the issue as selected for report

@C4-Staff C4-Staff added the M-02 label Jun 8, 2023
@c4-sponsor c4-sponsor added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Jul 7, 2023
@c4-sponsor
Copy link

drgorillamd marked the issue as sponsor acknowledged

@c4-sponsor c4-sponsor added sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") and removed sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons labels Jul 7, 2023
@c4-sponsor
Copy link

drgorillamd marked the issue as sponsor confirmed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue M-02 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

5 participants