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

0 stars 0 forks source link

Manipulation of premia through 0 transfer #615

Closed c4-bot-2 closed 10 months ago

c4-bot-2 commented 11 months ago

Lines of code

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

Vulnerability details

Impact

Its possible to transfer liquidity that only has the left slot occupied to manipulate the premia calculation of a target that does not have liquidity for that range yet, without any additional cost.

Proof of Concept

Providing liquidity and removing it through minting a long position allows any user to create a LeftRight-liquidity of x-0 (x being amount in left slot and 0 in right slot). Doing a zero-transfer of any of the tokens minted will assign this liquidity to any target address that does not yet have a position in that tickRange:

//SFPM
function registerTokenTransfer(address from, address to, uint256 id, uint256 amount) internal {
    ...
// Revert if recipient already has that position
if (
    (s_accountLiquidity[positionKey_to] != 0) ||
    (s_accountFeesBase[positionKey_to] != 0)
) revert Errors.TransferFailed();

// Revert if not all balance is transferred
uint256 fromLiq = s_accountLiquidity[positionKey_from];
if (fromLiq.rightSlot() != liquidityChunk.liquidity()) revert Errors.TransferFailed();

int256 fromBase = s_accountFeesBase[positionKey_from];

//update+store liquidity and fee values between accounts
s_accountLiquidity[positionKey_to] = fromLiq;
s_accountLiquidity[positionKey_from] = 0;

s_accountFeesBase[positionKey_to] = fromBase;
s_accountFeesBase[positionKey_from] = 0;
unchecked {
    ++leg;
}

Since the left slot, which represents the removed liquidity is important for the calculation of premia deltas, this can impact any upstream contracts that use them (e.g. the PanopticPool). This could for example take the form of frontrunning the initial mint on a tickRange of that pool to bypass the s_accountLiquidity[positionKey_to] != 0 check. That position would then start out with an amount x of supposedly removed liquidity.

Tools Used

Manual Review

Recommended Mitigation Steps

disallow transfers of positions with x-y liquidity where x != 0

Assessed type

Other

dyedm1 commented 10 months ago

dup #256

c4-sponsor commented 10 months ago

dyedm1 (sponsor) confirmed

c4-judge commented 10 months ago

Picodes marked the issue as duplicate of #256

c4-judge commented 10 months ago

Picodes marked the issue as satisfactory