Possible to sandwich attack users due to a lack of zero value check for `_minReceived`

[hats-bug-reporter[bot]]

Github username: @0xfuje Twitter username: 0xfuje Submission hash (on-chain): 0xefc70c7e5be5c58dffd0a76aea4ac1afc0328461394506f4c22ea1f3f7d69a05 Severity: medium

Description:

Impact

User funds can be lost due to the manipulations of reserves by the attacker

Description

There is a _minReceived input in convert(), buyPortalEnergy() and sellPortalEnergy() which would prevent an attacker from manipulating the reserves to profit in cases where the user specifies this value.

However it's possible and likely that less tech-savvy users will leave the _minReceived value at zero which would leave them vulnerable to a sandwich attack with buys and sells of portalEnergy & convert(). The current documentation does not highlight the impact of this input for the users.

Proof of Concept

1. Attacker monitors mempools for vulnerable Portal txs
2. User executes a large portal energy buy via buyPortalEnergy() with zero _minReceived
3. Attacker front-runs user's tx to buy PSM tokens with a flashloan and executes a huge buy of portalEnergy
4. This purchase raises the price of portalEnergy for the victim and increases the slippage
5. User buys portalEnergy at a much worse exchange rate than expected -> loses almost all of their funds (since there is no limit to slippage w zero _minReceived)
6. Attacker sells portalEnergy via sellPortalEnergy() for PSM profit from the user

Recommendation

Consider enforcing a zero value check in convert(), buyPortalEnergy() and sellPortalEnergy(). Consider to clearly document and warn users to specify a correct _minReceived value.

if (_minReceived == 0) {revert InvalidMinReceived();}

[PossumLabsCrypto]

Thanks for the submission but this is a double from #17