sherlock-audit / 2023-10-real-wagmi-judging

16 stars 14 forks source link

0xblackskull - Unsafe casting from `uint256` to `uint128` in LiquidityManager.sol #131

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

0xblackskull

medium

Unsafe casting from uint256 to uint128 in LiquidityManager.sol

Summary

Unsafe casting from uint256 to uint128 in LiquidityManager.sol

Vulnerability Detail

The vulnerability is present in _decreaseLiquidity for type casting value of amount0Min and amount1Min

Impact

Unsafe casting from uint256 to uint128 in a Solidity smart contract risks data loss, vulnerabilities, and unexpected behavior.

Code Snippet

function _decreaseLiquidity(uint256 tokenId, uint128 liquidity) private {
        // Call the decreaseLiquidity function of underlyingPositionManager contract
        // with DecreaseLiquidityParams struct as argument
        (uint256 amount0, uint256 amount1) = underlyingPositionManager.decreaseLiquidity(
            INonfungiblePositionManager.DecreaseLiquidityParams({
                tokenId: tokenId,
                liquidity: liquidity,
                amount0Min: 0,
                amount1Min: 0,
                deadline: block.timestamp
            })
        );
        // Check if both amount0 and amount1 are zero after decreasing liquidity
        // If true, revert with InvalidBorrowedLiquidity exception
        if (amount0 == 0 && amount1 == 0) {
            revert InvalidBorrowedLiquidity(tokenId);
        }
        // Call the collect function of underlyingPositionManager contract
        // with CollectParams struct as argument
        (amount0, amount1) = underlyingPositionManager.collect(
            INonfungiblePositionManager.CollectParams({
                tokenId: tokenId,
                recipient: address(this),
                amount0Max: uint128(amount0),   // @audit unsafe casting
                amount1Max: uint128(amount1)    // @audit unsafe casting
            })
        );
    }

Tool used

Manual Review

Recommendation

should check value before execution

require(amount0Min <= uint128(-1), "Value exceeds uint128 range");
require(amount1Min <= uint128(-1), "Value exceeds uint128 range");
sherlock-admin2 commented 11 months ago

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

This is not a problem. The function works as it should