sherlock-audit / 2024-05-beefy-cowcentrated-liquidity-manager-judging

5 stars 5 forks source link

merlin - A griefer can perform DOS attack on all major functions of the StrategyPassiveManagerVelodrome smart contract #121

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

merlin

medium

A griefer can perform DOS attack on all major functions of the StrategyPassiveManagerVelodrome smart contract

Summary

The deposit, withdraw, moveTicks, setPositionWidth, and unpause functions all call the internal _addLiquidity function, which interacts with the mint() function from the NftPositionManager smart contract.

Vulnerability Detail

Here is the internal _mintPosition function from StrategyPassiveManagerVelodrome smart contract, which will always fail due to griefers.

(uint256 nftId,,,) = INftPositionManager(nftManager).mint(mintParams);

Let's analyze the NonfungiblePositionManager.mint function. If we look at the end of the function, it calls refundETH, which sends any remaining ETH back to msg.sender:

/// contract NonfungiblePositionManager
function mint(MintParams calldata params)
        external
        payable
        override
        checkDeadline(params.deadline)
        returns (uint256 tokenId, uint128 liquidity, uint256 amount0, uint256 amount1)
    {
        ///Code

        refundETH();

        emit IncreaseLiquidity(tokenId, liquidity, amount0, amount1);
    }
 ///

 /// @inheritdoc IPeripheryPayments
    function refundETH() public payable override nonReentrant {
        if (address(this).balance > 0) TransferHelper.safeTransferETH(msg.sender, address(this).balance);
    }

function safeTransferETH(address to, uint256 value) internal {
        (bool success,) = to.call{value: value}(new bytes(0));
        require(success, "STE");
    }       

So, a griefer in this situation could simply send 1 wei of ETH to the NonfungiblePositionManager smart contract, and then all major functions of StrategyPassiveManagerVelodrome would be blocked.

Impact

All major functions of StrategyPassiveManagerVelodrome would be blocked.

Code Snippet

contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L321

Tool used

Manual Review

Recommendation

Consider adding a receive function to the StrategyPassiveManagerVelodrome smart contract.

Duplicate of #69

sherlock-admin3 commented 2 months ago

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

WildSniper commented:

not high due to the following quote, "Inflicts serious non-material losses (doesn't include contract simply not working". And a valid medium due to this quote, "Breaks core contract functionality, rendering the contract useless or leading to loss of funds." really nice catch