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

bonusAmounts is representing assets and shares at the same time in the liquidate function #44

Closed
c4-bot-8 opened this issue Apr 9, 2024 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly invalid This doesn't seem right withdrawn by warden Special case: warden has withdrawn this submission and it can be ignored

Comments

@c4-bot-8
Copy link
Contributor

c4-bot-8 commented Apr 9, 2024

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1165-L1168

Vulnerability details

Impact

bonusAmounts is representing assets and shares at the same time in the liquidate function

In the liquidate function from the PanopticPool contract, the output bonusAmounts is used as if it was representing a value of asset amounts and share amounts at the same time; which is impossible as there is a conversion rate between them. This error will make the liquidate function behave in an expected manner, not the one that was intended.

Proof of Concept

The issue is occurring in the liquidate function below:

PanopticPool.sol#L1165-L1168


@>      LeftRightSigned bonusAmounts = LeftRightSigned
            .wrap(0)
            .toRightSlot(int128(liquidationBonus0))
            .toLeftSlot(int128(liquidationBonus1));

As you can see from the code above, the value of the output bonusAmounts is used to represent either an asset amount or a share amount; which is impossible as there is a conversion formula used to transform the asset amount into the share amount (and inversely) with the function convertToShares (or convertToAssets). The vulnerability and it's impact is similar to PoolTogether H-03 where the _amountOut is representing assets and shares at the same time in the liquidate function.

Recommended Mitigation Steps

To solve this issue, I recommend to first convert the value of bonusAmounts in the liquidate function to an asset amount and store it in a local variable, bonusAmountsToAsset. In the function logic, use the correct variable, either bonusAmounts or bonusAmountsToAsset, when interacting with a share amount or an asset amount.

Assessed type

Error

@c4-bot-8 c4-bot-8 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Apr 9, 2024
c4-bot-3 added a commit that referenced this issue Apr 9, 2024
@c4-bot-8 c4-bot-8 added invalid This doesn't seem right withdrawn by warden Special case: warden has withdrawn this submission and it can be ignored and removed bug Something isn't working labels Apr 12, 2024
@c4-bot-10
Copy link
Contributor

Withdrawn by lightoasis

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 invalid This doesn't seem right withdrawn by warden Special case: warden has withdrawn this submission and it can be ignored
Projects
None yet
Development

No branches or pull requests

2 participants