_getAccountMargin can be manipulated

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1159-L1193
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L370-L374
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L379-L381
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L386-L388

Vulnerability details

Impact

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1159-L1193

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L386-L388

The attacker manipulates the historical positions they hold, or completely manipulates the total premium collected from all of their positions, they can make the use of the system unprofitable for the user. As a result, the user can lose their assets significantly. By manipulating the collateral status and margin details, the attacker can put the user's account at risk of a margin call. This can result in the liquidation of the user's position or restriction of access to their funds.

Proof of Concept

When creating a pool, _getAccountMargin to calculate the collateral status and margin details of the account/user. an array of historical positions the user has held, and the total premium collected from all positions. This way an attacker can make usage unprofitable in the first place.

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1159-L1193

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L370-L374

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L379-L381

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L386-L388

Tools Used

Manual Review

Recommended Mitigation Steps

There are different approaches on how to solve the Inflation Attack problem. Some of the approaches along with their pros and cons, can be found in the OpenZeppelin github issue: OpenZeppelin/openzeppelin-contracts#3706.

One way to resolve the problem is to use virtual dead shares, as implemented in the latest OpenZeppelin ERC-4626 vault:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/51294b7480fd13e716207a621ac1d55a6290d56d/contracts/token/ERC20/extensions/ERC4626.sol#L200
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/51294b7480fd13e716207a621ac1d55a6290d56d/contracts/token/ERC20/extensions/ERC4626.sol#L207

Assessed type

Context

[c4-judge]

Picodes marked the issue as unsatisfactory:
Insufficient proof