Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manipulation of premia through 0 transfer #615

Closed
c4-bot-2 opened this issue Dec 11, 2023 · 4 comments
Closed

Manipulation of premia through 0 transfer #615

c4-bot-2 opened this issue Dec 11, 2023 · 4 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-256 satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@c4-bot-2
Copy link
Contributor

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

@c4-bot-2 c4-bot-2 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 11, 2023
c4-bot-10 added a commit that referenced this issue Dec 11, 2023
@dyedm1
Copy link
Member

dyedm1 commented Dec 17, 2023

dup #256

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Dec 17, 2023
@c4-sponsor
Copy link

dyedm1 (sponsor) confirmed

@c4-judge
Copy link
Contributor

Picodes marked the issue as duplicate of #256

@c4-judge
Copy link
Contributor

Picodes marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-256 satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

4 participants