code-423n4 / 2024-04-panoptic-findings

8 stars 3 forks source link

The position could be minted in the SFPM directly, bypassing important checks in the PanopticPool. #472

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L504-L527

Vulnerability details

Summary

I suppose it is small mistake that wasn't taked into an account, however the consequences could be enormous. The position in the Panoptic protocol is minted/burned via SFPM.sol which is the core contract. However, as i understand, that the user flow should begin in the PanopticPool where the mint/burn functionality is started. However, the user could call the mint/burnTokenizedPosition from the SFPM contract and bypass important security checks

Vulnerability details

Additionaly, it worth to mention that if we call the mint funciton directly it would mint the ERC1155 shares to us (msg.sender) and we could receive the flow via onERC1155Received, which is dangerous, that could result in the unintended behaviour

Let's take a look which security checks could be bypassed if we would call mintTokenizedPosition directly.

  1. _validatePositionList is skipped. Which ensures the corectness of the position hash
  2. _getSlippageLimits is skipped. Which verifies that tick low/max are correct
  3. Check that verifies the corectness of the pool -> If tokenId.poolId() != SFPM.getPoolId(address(s_univ3pool)) revert
  4. Check that verifies that position isn't minted yet If ((s_positionBalance[msg.sender][tokenId]) != 0) revert
  5. Also, the _addUserOption function is skipped, which stores option data in the pool contract
  6. Solvency validation (_validateSolvency) is bypassed

Since important security checks are bypassed which break the overall functionality of the protocol the vulnerability is high

Recommended Mitigation Steps

Ensure that the onlyPool modifier is set to the mint/burn function in the SFPM

Assessed type

Access Control

Picodes commented 5 months ago

This is by design

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Invalid