search

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

1 stars 1 forks source link

Use of assert() instead of require() #166

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

Contracts use assert() instead of require() in multiple places.

Per to Solidity’s documentation:

"Assert should only be used to test for internal errors, and to check invariants. Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix. Language analysis tools can evaluate your contract to identify the conditions and function calls which will cause a Panic.”

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/NestedFactory.sol#L299-L299

assert(amountSpent <= _inputTokenAmount - feesAmount); // overspent

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/NestedFactory.sol#L341-L341

assert(amountSpent <= _inputTokenAmounts[i]);

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/operators/ZeroEx/ZeroExOperator.sol#L42-L43

assert(amountBought > 0);
assert(amountSold > 0);

Recommendation

Change to require().

alcueca commented 2 years ago

I'm upgrading this to Low Risk because there is an actual loss to the users, and the potential for a massive headache for the protocol operators.