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

6 stars 4 forks source link

BkdEthCvx's _withdrawAll can be subject to sandwich attacks #208

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/BkdEthCvx.sol#L110

Vulnerability details

Impact

ConvexStrategyBase's withdrawAll uses Curve pool data to estimate accepted minimum amount, which is a subject to manipulation, allowing sandwich attacks.

Trades can happen at a manipulated price and end up receiving fewer tokens than current market price dictates.

Placing severity to medium as withdrawAll can pull substantial amount of funds, making sandwich attacks economically viable, while they result in a partial fund loss.

Proof of Concept

withdrawAll calls _withdrawAll for funds withdrawal:

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/ConvexStrategyBase.sol#L143-L148

_withdrawAll uses _minUnderlyingAccepted for slippage control:

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/BkdEthCvx.sol#L110

_minUnderlyingAccepted is based on _lpToUnderlying:

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/BkdEthCvx.sol#L148-L150

_lpToUnderlying is calculated from pool's get_virtual_price:

https://github.com/code-423n4/2022-04-backd/blob/main/backd/contracts/strategies/BkdEthCvx.sol#L171

Recommended Mitigation Steps

Consider adding minimum accepted return argument to the withdrawAll and condition execution success on it.

chase-manning commented 2 years ago

Curve's get_virtual_price view is not manipulatable