obront - Users will not be able to buy shares with predictable prices

[sherlock-admin]

obront

medium

Users will not be able to buy shares with predictable prices

Summary

It is generally considered bad practice to have prices read from state (including future oracles) without user input. This is because users are not able to be confident in any parameters on the prices they should expect when the trade clears. It's important to allow users to set some parameters to dictate the trades that they are willing to make.

Vulnerability Detail

When a user mints or redeems through the protocol, they are committing to buy or sell their shares at an unknown future rate. While this rate cannot be manipulated, thanks to the oracle system, there are the potential for large changes that put the price outside the range a user would want to execute at.

As an example:

• I see that the price of an asset on Float is X, and believe X is a great price
• I mint tokens in a 10X levered vault to take advantage of this great price
• Before the trade clears, the market moves by 1%
• When my trade clears, I have 10% less tokens than I expected for my investment

Impact

This has the potential to keep users from making large, confident bets, as user's are not able to make a clear bet or set a limit to the price at which they are willing to pay.

While this may seem like a small thing, numerous protocols have faced this problem and seen the consequences, which is why slippage parameters are included in user arguments.

Code Snippet

User quantity of tokens minted is based on price at later settlement time:

https://github.com/sherlock-audit/2022-11-float-capital/blob/main/contracts/market/template/MarketCore.sol#L448-L478

User amount of DAI redeemed is based on price at later settlement time:

https://github.com/sherlock-audit/2022-11-float-capital/blob/main/contracts/market/template/MarketCore.sol#L504-L532

Tool used

Manual Review

Recommendation

I understand this is a key part of how the protocol works, and wouldn't expect a totally different system that allows users to set their own prices.

However, most protocols (such as DEXs) that read from state allow users to input some slippage parameters to ensure trades are rejected if the state changes too beyond what they are comfortable with.

My recommendation is to add a maxPrice argument to the _mint() function, and a minPrice argument to the _redeem() function, which are then saved in userActions to be checked upon settlement. If a price has moved too far, the user's userActions can be cleared out and their deposit can be returned.

[JasoonS]

This is the core premise of how the protocol works.

It must be noted that the user isn't effected by the price update that they get in at.

If they put 100 DAI in, they'll have EXACTLY 100DAI worth of pool tokens at the current (latest) known price at the time the system state is updated. (minus fees of course)

There is absolutely no slippage.

We are not an options protocol - we can't only execute trades for users if the price goes down/up - for that they shouldn't use Float.