c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategyCore.sol#L233-L240 https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategyCore.sol#L258-L263

Vulnerability details

Bug Description

In VotiumStrategyCore.sol, the buyCvx() and sellCvx() functions call exchange_underlying() of Curve's ETH / CVX pool to buy and sell CVX respectively:

VotiumStrategyCore.sol#L233-L240

        ICrvEthPool(CVX_ETH_CRV_POOL_ADDRESS).exchange_underlying{
            value: _ethAmountIn
        }(
            0,
            1,
            _ethAmountIn,
            0 // this is handled at the afEth level
        );

VotiumStrategyCore.sol#L258-L263

        ICrvEthPool(CVX_ETH_CRV_POOL_ADDRESS).exchange_underlying(
            1,
            0,
            _cvxAmountIn,
            0 // this is handled at the afEth level
        );

As seen from above, exchange_underlying() is called with its _min_dy parameter as 0, which means the minimum amount of CVX or ETH to receive from the swap is effectively 0.

This isn't an issue when users interact with the AfEth contract, as its deposit() and withdraw() functions include a _minOut parameter which protects against slippage.

However, users that interact with the VotiumStrategy contract directly will not be protected from slippage when they call any of the following functions:

deposit(), which calls buyCvx()
depositRewards(), which calls buyCvx()
withdraw(), which calls sellCvx()

Should users call any of the functions listed above directly, they will be susceptible to sandwich attacks by attackers, which would reduce the amount of CVX or ETH received from the swap with curve's pool.

Impact

Due to a lack of slippage protection in buyCvx() and sellCvx(), users that interact with the VotiumStrategy contract will be susceptible to sandwich attacks. This results in a loss of funds for them as they will receive less CVX or ETH for the same amount of funds.

Proof of Concept

Consider the following scenario:

Bob calls the VotiumStrategy contract's deposit() function directly to deposit ETH.
Alice sees his transaction in the mempool and front-runs his transaction. She swaps a large amount of ETH into the Curve pool and gets CVX in return.
Now, Bob's transaction is executed:
- buyCvx() attempts to swap Bob's ETH deposit for CVX.
- However, since the pool currently has a lot more ETH than CVX, Bob only gets a small amount of CVX in return.
Alice back-runs Bob's transaction and swaps the CVX she gained for ETH in the pool, which would result in a profit for her.

In this scenario, Alice has sandwiched Bob's deposit() transaction for a profit, causing Bob to receive less CVX for his deposited ETH.

Recommended Mitigation

Consider adding a _minOut parameter to either buyCvx() and sellCvx(), or the following functions:

This allows the caller to specify a minimum amount they expect from the swap, which would protect them from slippage.

Assessed type

MEV

elmutt commented 1 year ago

@toshiSat i think we should just lock this down so afEth can only use votium strategy

elmutt commented 1 year ago

https://github.com/asymmetryfinance/afeth/pull/169