code-423n4 / 2021-11-nested-findings

1 stars 1 forks source link

Function NestedFactory._submitOutOrders doesn't check amounts returned from market executing of user's orders #196

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

hyh

Vulnerability details

Impact

Malicious user can track market order and perform sandwich attack whenever order amount to be executed on an exchange is big enough to compensate for exchange pool manipulation costs.

Proof of Concept

Function NestedFactory._submitOutOrders is used in user facing functions NestedFactory.sellTokensToWallet and sellTokensToNft.

_submitOutOrders doesn't check amounts returned after executing user's orders, this way accepting any amount returned, thus is vulnerable.

_submitOutOrders: https://github.com/code-423n4/2021-11-nested/blob/main/contracts/NestedFactory.sol#L321

sellTokensToWallet: https://github.com/code-423n4/2021-11-nested/blob/main/contracts/NestedFactory.sol#L165

sellTokensToNft: https://github.com/code-423n4/2021-11-nested/blob/main/contracts/NestedFactory.sol#L188

Recommended Mitigation Steps

Returned amountBought should be checked against user provided minimum viable return amount, for example, minAmount. This amount to be added as another argument to the corresponding user facing functions and passed to _submitOutOrders, which should check the amounts:

require(amountBought >= minAmount, "NestedFactory::_submitOutOrders: Slippage too big");
maximebrugel commented 2 years ago

Duplicated : #86