search

code-423n4 / 2023-11-panoptic-findings

0 stars 0 forks source link

Long re-orgs risk loss of user funds, fees, and corrupt system state. #422

Closed c4-bot-1 closed 10 months ago

c4-bot-1 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L288-L290 https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L296

Vulnerability details

Impact

The SFPM contract accumulates and checkpoints values like fees, liquidity, and premiums in its state storage variables. For example:

// Accumulates owed fees for removed liquidity
mapping(bytes32 => uint256) private s_accountPremiumOwed 

// Accumulates total fees for added liquidity  
mapping(bytes32 => uint256) private s_accountPremiumGross

// Tracks net and removed liquidity
mapping(bytes32 => uint256) internal s_accountLiquidity

These values accumulate on-chain per user over time. The contract relies on the persisted state across transactions.

If the underlying blockchain experiences a long re-org and the SFPM contract state is reverted to an earlier point:

Scenario

  1. Bob deposits tokens and mints positions over 1 week, earning fees
  2. At the end of the week, a long 14 block re-org happens on the chain
  3. SFPM contract state reverts to earlier point before Bob's transactions
  4. Bob's positions and earned fees disappear from contract storage

No storage checkpoints to allow contract state recovery on long re-orgs, so User funds and accumulated values can disappear without recourse during long re-orgs. Would lead to broken system state.

Proof of Concept

Persisted user balances and positions can disappear without user recourse, leading to a broken platform state.

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L288-L290

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L296

As these persisted variables accumulate on-chain and are relied on across transactions, a long reorganization that reverted these mappings to much older states could corrupt user balances and positions.

The risk of this occurring in case of a deep chain re-org, where the contract state reverts to a previous checkpoint, but user expectations and actual deposited amounts are now different. This could break internal accounting and make the system untenable.

Tools Used

Vs Code

Recommended Mitigation Steps

Implement regular state checkpoints in critical state variables.

Assessed type

Other

c4-judge commented 10 months ago

Picodes marked the issue as unsatisfactory: Invalid