search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

Position owner should set allowed slippage #87

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/TopUpAction.sol#L154 https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/TopUpAction.sol#L187

Vulnerability details

Impact

The default swap slippage of 5% allows malicious keepers to sandwich attack topup. Additionally, up to 40% (_MIN_SWAPPER_SLIPPAGE) slippage allows malicious owner to sandwich huge amounts from topup

Proof of Concept

Keeper can bundle swaps before and after topup to sandwich topup action, in fact it's actually in their best interest to do so.

Tools Used

Recommended Mitigation Steps

Allow user to specify max swap slippage when creating topup similar to how it's specified on uniswap or sushiswap to block attacks from both keepers and owners

gzeoneth commented 2 years ago

According to C4 Judging criteria

Unless there is something uniquely novel created by combining vectors, most submissions regarding vulnerabilities that are inherent to a particular system or the Ethereum network as a whole should be considered QA. Examples of such vulnerabilities include front running, sandwich attacks, and MEV.

However since Backd use keeper to run topup transactions, which presumably are bots and smart contracts that can fetch onchain price directly. A large (5% default, up to 40%) seems excessive and can lead to user losing fund. Judging this as Med Risk.

chase-manning commented 2 years ago

https://github.com/backdfund/protocol/pull/297