search

FloorDAO / floor-v2

Floor aims to create a fully onchain governance mechanism for sweeping and deploying NFTs to profitable NFT-Fi strategies as well as seeding liquidity for its own NFT-Fi products.
https://floor.xyz
2 stars 0 forks source link

[UVS-04M] Inexistent Slippage Parameters #111

Closed tomwade closed 1 year ago

tomwade commented 1 year ago

UVS-04M: Inexistent Slippage Parameters

Type Severity Location
Logical Fault UniswapV3Strategy.sol:L260

Description:

The UniswapV3Strategy::withdrawPercentage function will invoke the UniswapV3Strategy::_withdraw function with a value of 0 for both token minimum outputs, rendering the interaction significantly insecure and prone to slippage attacks.

Impact:

As a strategy is expected to deal with significant liquidity, passing in zero values for the minimum desired outputs of the liquidity's extraction is prone to what are known as sandwich attacks which will execute a set of operations before and after the liquidity withdrawal, diminishing the actual value withdrawn maliciously.

Example:

function withdrawPercentage(address recipient, uint percentage) external override onlyOwner returns (address[] memory, uint[] memory) {
    // Get our liquidity for our position
    (,,,,,,, uint liquidity,,,,) = positionManager.positions(tokenId);

    // Call our internal {withdrawErc20} function to move tokens to the caller. Pulling out
    // LP or adding it doesn't have sandwhiches like trading so providing 0 in both places
    // should just work.
    return _withdraw(recipient, 0, 0, block.timestamp, uint128((liquidity * percentage) / 10000));
}

Recommendation:

We advise either an on-chain oracle to be utilized or the percentage-based withdrawal flow to not be supported by the system as it is presently susceptible to fund loss due to slippage.