Platform: Code4rena
Start Date: 04/06/2024
Pot Size: $25,000 USDC
Total HM: 0
Participants: 3
Period: 6 days
Judge: Picodes
Id: 387
League: ETH
Rank: 2/3
Findings: 1
Award: $339.47
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: sammy
Also found by: Bauchibred, bigtone
339.4737 USDC - $339.47
https://github.com/code-423n4/2024-06-panoptic/blob/main/contracts/CollateralTracker.sol#L578
The lack of overflow validation allows s_poolAssets to be manipulated. Once overflow occurs, totalAssets can be set higher than the actual collaterals, preventing other users from withdrawing their own collateral due to the incorrect totalAssets.
totalAssets
is calculated as the sum of s_poolAssets
and s_inAMM
.
shares = assets * totalSupply / totalAssets() assets = shares / totalSupply * (s_poolAssets + s_inAMM)
If a user owns 50% of the totalShares
, their withdrawal assets are calculated as:
assets = 0.5 * (s_poolAssets + s_inAMM)
If s_inAMM
is significantly larger than s_poolAssets
, the calculated assets can exceed s_poolAssets
, leading to an overflow of s_poolAssets
.
assets = 0.5 * (s_poolAssets + s_poolAssets + x)
s_poolAssets
and s_inAMM
are calculated in the takeCommissionAddData
function.
File: https://github.com/code-423n4/2024-06-panoptic/blob/main/contracts/CollateralTracker.sol#L578 function withdraw( uint256 assets, address receiver, address owner, TokenId[] calldata positionIdList ) external returns (uint256 shares) { shares = previewWithdraw(assets); // check/update allowance for approved withdraw if (msg.sender != owner) { uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals. if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares; } // burn collateral shares of the Panoptic Pool funds (this ERC20 token) _burn(owner, shares); // update tracked asset balance unchecked { s_poolAssets -= uint128(assets); // @audit assets can be larger than s_poolAssets? } ... }
Manual review
Add overflow validation or remove the unchecked to prevent manipulation of s_poolAssets.
function withdraw( uint256 assets, address receiver, address owner, TokenId[] calldata positionIdList ) external returns (uint256 shares) { + if (assets > s_poolAssets) revert Errors.ExceedsMaximumRedemption(); shares = previewWithdraw(assets); ... }
Under/Overflow
#0 - c4-judge
2024-06-13T16:45:29Z
Picodes marked the issue as duplicate of #38
#1 - c4-judge
2024-06-13T17:44:24Z
Picodes changed the severity to QA (Quality Assurance)
#2 - c4-judge
2024-06-13T17:44:55Z
Picodes marked the issue as grade-b
#3 - liveactionllama
2024-06-20T17:47:12Z
Per direction from the judge, staff have marked as 1st place. Also noting there was a tie for 1st/2nd place.