BasePositionManager Manual Review Findings

BPM-01M: Insecure Payment Performance (Permanent Queue DoS)

Type	Severity	Location
Logical Fault		BasePositionManager.sol:L91

Description:

The BasePositionManager::_transferOutETHWithGasLimitFallbackToWeth function will attempt to perform a "safe" native transfer towards the _receiver, however, the mechanism is presently insecure.

Specifically, the _receiver can execute what is known as a "return bomb" attack whereby the returned payload has a significantly high "length" that will force the caller contract to fail when decoding the bytes memory data due to being unable to reserve the necessary amount of memory.

In the context of the PositionManager, the BasePositionManager::_transferOutETHWithGasLimitFallbackToWeth function is invoked to a user-controlled _receiver in the following three instances:

• PositionManager::cancelOpenPosition w/ isDepositedInETH flag true
• PositionManager::executeClosePosition w/ withdrawETH flag true
• PositionManager::settlePosition w/ _withdrawETH flag true

The latter of the three is of no importance as it is externally invoked and its failure would not impact the operational security of the Moby project. The former two, however, can be exploited to force a permanent Denial-of-Service.

We can analyse that the PositionManager::cancelOpenPosition function is solely invoked when the original operation's PositionManager::executeOpenPosition function failed. A malicious user can force failure of this execution by reverting on their EIP-1155 receive hook of the Option token as evidenced in Controller::pluginOpenPosition.

Afterwards, the position's closure will result in failure when attempting to refund the malicious user thereby compromising the PositionManager queue and preventing it from ever being executed again.

Similarly and in a more straightforward fashion, a PositionManager::executeClosePosition function can cause failure by simply reverting on the yielded native funds that are the proceeds of the position's closure.

Impact:

It is possible to compromise the queue system the Moby project relies on by crafting malicious requests in the PositionManager that will never be executable and thus cause the queue to halt.

Example:

Copy
87function _transferOutETHWithGasLimitFallbackToWeth(uint256 _amountOut, address payable _receiver) internal {
88    IWETH _weth = IWETH(weth);
89    _weth.withdraw(_amountOut);
90
91    (bool success, /* bytes memory data */) = _receiver.call{ value: _amountOut, gas: ethTransferGasLimit }("");
92
93    if (success) { return; }
94
95    // if the transfer failed, re-wrap the token and send it to the receiver
96    _weth.deposit{ value: _amountOut }();
97    _weth.transfer(address(_receiver), _amountOut);
98}

Recommendation:

We advise the payment mechanism to be made secure by performing it in a low-level assembly block, ignoring the returned data. This can be achieved via a wrapper library such as ExcessivelySafeCall in a re-usable manner.

Alleviation (b02fae335f62cc1f5f4236fb4d982ad16a32bd26):

The code was updated to perform an assembly native transfer ignoring the call's output and thus alleviating this exhibit.