[UVS-02M] Potentially Unsafe Cast

[tomwade]

UVS-02M: Potentially Unsafe Cast

Type	Severity	Location
Mathematical Operations		UniswapV3Strategy.sol:L260

Description:

The referenced uint128 casting operation is performed without any upper-bound limitations. Coupled with the fact that the percentage is not sanitized, it is possible to set an abnormally high percentage that would cause a casting overflow to occur and the UniswapV3Strategy::withdrawPercentage function to actually succeed when it should have failed.

Impact:

While the overflow operation would cause the withdrawal to succeed with an abnormally high percentage, the exhibit's impact depends on how the UniswapV3Strategy::withdrawPercentage function is utilized.

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 the code to either sanitize the percentage argument to be at most 100_00 or to perform the uint128 type cast safely, either of which we consider an adequate resolution to this exhibit.

[tomwade]

Our StrategyFactory validates the percentage before it goes to our strategy contracts, so this parameter should already be checked.

// Ensure our percentage is valid (less than 100% to 2 decimal places)
require(_percentage > 0, 'Invalid percentage');
require(_percentage <= 100_00, 'Invalid percentage');