The CollateralTracker.takeCommissionAddData function and CollateralTracker.exercise functions are used to take the commission amount during option creation and exercise an option and pay to the seller what is owed from the buyer, respectively.
Both the above two functions update the s_inAMM (The total assets in the UniswapV3Pool related to the Panoptic protocol) value at the end of their function execution to account for the updated asset values as a result of the operations as shown below:
The issue here is that int256 to uint256 type casting is unsafe and could result in erroneous value thus breaking the state of the s_inAMM value.
Let's assume the following scenario:
The CollateralTracker contract has been deployed and the s_inAMM is initially set to 0 (This would be the case when the s_inAMM is a small value when enough funds are transferred to the UniswapV3 pool at the beginning) .
A malicious user front-runs other transactions with a PanopticPool.mintOptions transaction, which makes a subsequent call to the CollateralTracker.takeCommissionAddData function such that shortAmount > longAmount.
As a result the (shortAmount - longAmount) value is > 0 for the current option being minted.
Since the s_inAMM == 0 the int256(uint256(s_inAMM)) - (shortAmount - longAmount) will result in a negative value and this negative value is represented in two's complement.
For example the -1 will be represented with all bits set to 1 in the int256 binary representation.
The issue is when this gets casted to uint256 this two's complement of negative representation is not taken into account and directly casts it to uint256. Hence if the converted value is -1 then the converted value into uint256 will be the type(uint256).max which gets downcast to the uint128 and as a result the s_inAMM will have the maximum value of type(uint128).max.
As a result the s_inAMM enters into an erroneous and broken state that can not be rectified and will negatively affect the subsequent critical transactions of the panoptic protocol as described below.
This could prompt all the subsequent operations which uses the totalAssets() function such as previewDeposit, previewMint, previewWithdraw, _poolUtilization, revoke, takeCommissionAddData, excercise functions performing erroneous operations (with wrong totalAssets() returned value) which results in broken state of the panoptic protocol.
Further if the -1 is casted into uint256 then since the s_inAMM is assigned type(uint128).max value any subsequent addition to the s_inAMM will result in transaction revert due to uint128 overflow. This will DoS the CollteralTracker.excercise and CollateralTracker.takeCommissionAddData thus breaking the mint and exercise functionalities of the PanopticPool contract.
This vulnerability is highly probable at the initial stage of the CollateralTracker deployment since at the beginning not enough assets are transferred to the UniswapV3 pool and as a result s_inAMM is comparatively a smaller value thus can underflow easily.
Hence it is recommended to implement safeCast for the int256 to uint256 casting and revert the transaction if a negative value of int256 is being casted into uint256 value. Hence this will prevent the s_inAMM state entering into an erroneous and broken state.
Lines of code
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1085 https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1029
Vulnerability details
Impact
The
CollateralTracker.takeCommissionAddData
function andCollateralTracker.exercise
functions are used to take the commission amount during option creation and exercise an option and pay to the seller what is owed from the buyer, respectively.Both the above two functions update the
s_inAMM
(The total assets in the UniswapV3Pool related to the Panoptic protocol) value at the end of their function execution to account for the updated asset values as a result of the operations as shown below:The issue here is that
int256 to uint256
type casting is unsafe and could result in erroneous value thus breaking the state of thes_inAMM
value.Let's assume the following scenario:
CollateralTracker
contract has been deployed and thes_inAMM
is initially set to 0 (This would be the case when the s_inAMM is a small value when enough funds are transferred to the UniswapV3 pool at the beginning) .PanopticPool.mintOptions
transaction, which makes a subsequent call to theCollateralTracker.takeCommissionAddData
function such thatshortAmount > longAmount
.(shortAmount - longAmount)
value is> 0
for the current option being minted.s_inAMM == 0
theint256(uint256(s_inAMM)) - (shortAmount - longAmount)
will result in a negative value and this negative value is represented in two's complement.-1
will be represented with all bits set to1
in theint256
binary representation.uint256
this two's complement of negative representation is not taken into account and directly casts it touint256
. Hence if the converted value is-1
then the converted value into uint256 will be thetype(uint256).max
which gets downcast to the uint128 and as a result thes_inAMM
will have the maximum value oftype(uint128).max
.s_inAMM
enters into an erroneous and broken state that can not be rectified and will negatively affect the subsequent critical transactions of the panoptic protocol as described below.This could prompt all the subsequent operations which uses the
totalAssets()
function such aspreviewDeposit, previewMint, previewWithdraw, _poolUtilization, revoke, takeCommissionAddData, excercise
functions performing erroneous operations (with wrong totalAssets() returned value) which results in broken state of the panoptic protocol.Further if the
-1
is casted intouint256
then since thes_inAMM
is assignedtype(uint128).max
value any subsequent addition to thes_inAMM
will result in transaction revert due to uint128 overflow. This willDoS
theCollteralTracker.excercise
andCollateralTracker.takeCommissionAddData
thus breaking themint and exercise
functionalities of thePanopticPool
contract.This vulnerability is highly probable at the initial stage of the
CollateralTracker
deployment since at the beginning not enough assets are transferred to theUniswapV3 pool
and as a results_inAMM
is comparatively a smaller value thus can underflow easily.Proof of Concept
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1085
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1029
Tools Used
Manual Review and VSCode
Recommended Mitigation Steps
Hence it is recommended to implement safeCast for the
int256 to uint256
casting and revert the transaction if a negative value ofint256
is being casted intouint256
value. Hence this will prevent thes_inAMM
state entering into an erroneous and broken state.Assessed type
DoS