Pumponomics can be skipped when using FluidLocker::provideLiquidity

Summary

Pumponomics can be effectively skipped since the eth amount used to create a position on uniswap could be different than the called amount. It uses all available balance inside the contract. This causes the intended buy-pressure mechanism to be skipped, and damages Superfluid's economic model.

Root Cause

Inside FluidLocker::provideLiquidity function, only the eth sent in this call are pumped. However, all available wrapped eth are used to create a position. This allows locker owners to send eth manually to the locker, not when they are calling the provideLiquidity function, effectively skipping Pumponomics.

Internal Pre-conditions

None

External Pre-conditions

None

Attack Path

1. Locker owner transfers certain amount of wrapped ethers to the locker directly by calling WETH9.transfer, they will be used as the paired asset for provideLiquidity function.
2. Call provideLiquidity function with a dust amount of eth, and a normal amount of supAmount to utilize the wrapped ether we sent in step 1. Now a position is created without Pumponomics.

Impact

Weaken the intended structural buy-pressure; causes protocol to lose potential fees earned from swaps and liquidity.

PoC

Due to the difficulty of observing the effect of the pump function (1% difference), we directly modify the FluidLocker contract for visibility by creating a new field uint256 public ethPumped; and place it after all other fields that already exist. To use it, we insert this line into the beginning of the _pump function ethPumped += ethAmount; These modification should not change the behaviour of the contract.

Now, paste the following test into FluidLockerTest contract inside FluidLocker.t.sol If needed, please import import { IWETH9 } from "../src/token/IWETH9.sol";

    function testSkipPumponomics()
    external
    virtual
    {
        uint fundingAmount = 100e18;
        // Set up Alice's Locker to be functional
        // i. e. not revert due to "lack of funds", "no LP pool units", "no staker pool units", etc.
        _helperFundLocker(address(aliceLocker), fundingAmount);
        _helperLockerStake(address(bobLocker));
        _helperLockerProvideLiquidity(address(carolLocker));

        //Alice transfer the eth into locker via a plain call
        //Then call provideLiquidity with dust amount of eth
        //Therefore the pump function only pumps the dust eth amount, but the position is created with all eth in locker
        uint ethAmount = fundingAmount / (2 * 9900);
        address weth = _nonfungiblePositionManager.WETH9();
        vm.startPrank(ALICE);
        IWETH9(weth).deposit{ value: ethAmount }();
        IWETH9(weth).transfer(address(aliceLocker), ethAmount);
        aliceLocker.provideLiquidity{ value: 100 }(fundingAmount);
        vm.stopPrank();

        // Only a dust amount of eth has been pumped
        assertEq(FluidLocker(payable(address(aliceLocker))).ethPumped(), 1);
        // However, a position is opened with basically all the eth in the locker
        assertGt(FluidLocker(payable(address(aliceLocker))).activePositionCount(), 0);
        assertApproxEqAbs(0,
            IWETH9(weth).balanceOf(address(aliceLocker)),
            fundingAmount * 5 / 100 // 5% tolerance
        );
    }