hats-finance / Possum-Labs--Portals--0xed8965d49b8aeca763447d56e6da7f4e0506b2d3

GNU General Public License v2.0
0 stars 2 forks source link

Portal Ignores Principal Token and PSM Price Ratio #49

Open hats-bug-reporter[bot] opened 11 months ago

hats-bug-reporter[bot] commented 11 months ago

Github username: @0xmahdirostami Twitter username: 0xmahdirostami Submission hash (on-chain): 0x01dd911fdbaddd77f27734b310017dba8ff282920fcde96fd57a222410e1b304 Severity: high

Description: Description\

The current implementation of the portal does not consider the price ratio between the principal token and PSM when calculating portal energy. This can lead to a situation where the yield is significantly less compared to the staked amount, especially when the PSM price is lower than the principal token price.

Picture this: currently PSM price is 0.001, and HLP price is 1.02. If a user deposits 1000e18 HLP, they'll get around 250e18 energy. Now, the energy converts to PSM at a rate of 550:1, so they end up with nearly 5e17 PSM tokens, valued at 0.0005.

Impact\ The current implementation may result in users receiving less yield than expected, causing a discrepancy between the staked amount and the obtained yield

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

    Suggested fix: Consider both the principal token price and PSM price when using the _FUNDING_EXCHANGE_RATIO.

PossumLabsCrypto commented 11 months ago

First of all, great find!

The discrepancy originates from a faulty constantProduct calculation when calling activatePortal().

uint256 requiredPortalEnergyLiquidity = fundingBalance * FUNDING_EXCHANGE_RATIO; constantProduct = fundingBalance * requiredPortalEnergyLiquidity;

Instead of multiplication there should be division; uint256 requiredPortalEnergyLiquidity = fundingBalance / FUNDING_EXCHANGE_RATIO;

To preserve precision, the formula should be condensed to this: constantProduct = fundingBalance**2 / FUNDING_EXCHANGE_RATIO;

It seems like the wrong FUNDING_EXCHANGE_RATIO slipped in due to a confusion of the denominating currency.

As a result, PE would have been severely underpriced, which can be fixed in multiple ways even after contract deployment, the easiest being to buy loads of PE with PSM from the treasury. Afterwards, the Portal would work just fine.

Therefore, assigning a medium severity seems appropriate.

Issues that lead to an economic loss but do not lead to direct loss of on-chain assets.

Though I want to emphasize that this is a very valuable finding and I´m happy to award you with additional out-of-competition rewards of 500k PSM.

Thanks a lot for saving us this headache. Although we´ll tripple check all calculations before launching on mainnet, who knows if we would have catched this. 🤝

0xmahdirostami commented 11 months ago

Dear @PossumLabsCrypto,

Thank you for providing your valuable feedback. I would like to contribute some comments to your suggestion, as I strongly believe this is a high-priority issue:

I also reviewed your revised code and noticed another issue in the current implementation that I submitted in issue #60 .

PossumLabsCrypto commented 11 months ago

Thank you ser, duly noted.

To keep the competition fair, our judgement process must be consistent. Given that our frontend will show APR % on launch, this mispricing would be discovered immediately by users and ourselves, if not discovered before launch. But these are hypothetical scenarios anyways that are not worth debating.

The bug does not allow an external attacker to get access to user funds or irreversibly lose them which is the underlying principle of a high severity. As outlined above, the description of a medium severity fits this finding perfectly and we acknowledge the extra importance with an additional reward instead of diluting the findings of everyone else by grading it as high severity.

I trust in your understanding that the consistent and consequent application of severity grading is for the benefit of all participants including yourself. 🤝 🙂