View functions are vulnerable to front running attacks by miners

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L272-L304 https://github.com/code-423n4/2023-08-shell/blob/c61cf0e01bada04c3d6055acb81f61955ed600aa/src/proteus/EvolvingProteus.sol#L312-L344

Vulnerability details

Impact

Users would get worse rates than expected due to miner frontrunning.

Proof of Concept

The view functions like swapGivenInputAmount and swapGivenOutputAmount return calculated swap values, but do not update state. A malicious miner could exploit this by the following steps:

1. User sends a transaction calling swapGivenInputAmount to preview the swap.
2. The miner sees this transaction in the mempool.
3. Before the user's transaction is mined, the miner sends their own transaction with the same input as the user, calling the actual swap function.
4. The miner's transaction will get mined first since they control block production. This allows them to get the swap output previewed by the user.
5. When the user's transaction finally gets mined, the state has already changed due to the miner's transaction, so the user gets a worse rate than previewed.

Tools Used

Manual

Recommended Mitigation Steps

1. Make view functions private or internal so they cannot be called externally.
2. Use a commit-reveal scheme where user submits hash of swap params that is revealed later.
3. Check that swap parameters match last previewed values.

Assessed type

Other

[c4-pre-sort]

0xRobocop marked the issue as primary issue

[c4-pre-sort]

0xRobocop marked the issue as low quality report

[0xRobocop]

Invalid

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid