Protocol requires a running bot in order to make sure trades are actually executed

[code423n4]

Handle

tensors

Vulnerability details

Impact

Because smart contracts need to be poked to execute, trades placed before an oracle update won't be executed until someone else calls the function to execute queued trades. This means that a bot must run to constantly execute trades after every oracle update.

If such a bot was not running, users would have an incentive to only execute their trades after a favorable oracle update. However, having a dedicated bot run by the team centralizes the project with a single failure point. The typical solution here is to create keeper incentives for the protocol.

Proof of Concept

https://github.com/code-423n4/2021-08-floatcapital/blob/bd419abf68e775103df6e40d8f0e8d40156c2f81/contracts/contracts/LongShort.sol#L168

Recommended Mitigation Steps

Either make sure the team has a bot running or preferably create incentives for other users to constantly keep the queued orders executing.

[JasoonS]

0 - non-critical

In the worst case of the bot going down, no vulnerabilities open up.

The only issue that arises is that the price won't be tracked as accurately during those periods (but it will still track it, just not as accurately).

The protocol allows anyone to call an update, and any user interaction (non-update interaction) will also call the update if an update is due.

If such a bot was not running, users would have an incentive to only execute their trades after a favorable oracle update.

That is great, then the incentives are working, they cannot do better than a fair price update which is 'fair' in my opinion.

Indeed: We have been in talks with the chainlink keeprs team to be the first project to use them when they launch on polygon. We should have mentioned this in the README. Until then we have built a robust bot.

Note, this bot is for UX, not to patch up a vulnerability.

[moose-code]

Agree with @JasoonS . The bot failing opens no vulnerabilities, the synthetics may just less closely track the underlying if no contract interactions are present.

Given rational markets with actors on both sides (long and short), and given it will always be beneficial for one side to execute the update to capitalize on the movement, its safe to assume that in a rational market the updateSystemState in every case should be called by at least one participant, meaning theoretically a bot isn't even necessary.

[Stentonian]

My comments on the issue description

trades placed before an oracle update won't be executed until someone else calls the function to execute queued trades.

What function is this? There are multiple update functions so it's not clear which one the warden is referring to. If they are referring to the function _executeOutstandingNextPriceSettlements (the one that they linked in the 'Proof of concept' section) then the warden's statement is a non-issue. The system is designed to do exactly what the warden says. So the next statement is incorrect:

This means that a bot must run to constantly execute trades after every oracle update.

LongShort keeps track of which trade should happen at which price for each user, so when _executeOutstandingNextPriceSettlements is eventually called (no matter how many price updates have occurred since the trade request) it will use the oracle's next price update that occurred right after the time when trade was requestd. And _executeOutstandingNextPriceSettlements is called before any other action is taken by the user that would require the data from the trade having completed. No need to have a bot call this function.

If the warden is referring to another update function then it's not clear which.

---

My comments on @JasoonS's comment

The only issue that arises is that the price won't be tracked as accurately during those periods (but it will still track it, just not as accurately).

This is not actually mentioned by the warden in the issue description, even tho it is a real problem. So I don't think we need to include it here. The warden is specifically pointing out executeOutstandingNextPriceSettlements which will never have any issues no matter if there is a bot or not.

[0xean]

There are incentives built into the market already for users to update if the bot was to stop running. Yes, the bot should be treated as a tier-1 service for a reasonable user experience and for the most efficient tracking, but the risk of this to the system as a whole locking user funds or allowing people to significantly and unfairly profit doesn't seem to be there.

All that being said, it is a failure point within the system but one with low risk, downgrading to 1 - Low Risk.