assert() used instead of require()

[code423n4]

Handle

sirhashalot

Vulnerability details

Impact

The assert() and require() perform similar functions, but require() returns the remaining gas while assert() consumes the gas instead. This could lead to users wasting gas unnecessarily. Furthermore, require() allows for better debugging and troubleshooting if an error condition is unexpectedly reached. While assert() took on the same functionality as require() in Solidity 0.8.0 (see breaking changes: https://docs.soliditylang.org/en/v0.8.11/080-breaking-changes.html#silent-changes-of-the-semantics), the Yeti contracts use Solidity 0.6.11 and therefore assert() does not return gas upon failure.

https://ethereum.stackexchange.com/questions/15166/difference-between-require-and-assert-and-the-difference-between-revert-and-thro

Proof of Concept

This issue is prevalent in dozens of locations through the contracts, so only a subset of all instances are listed below.

YUSDToken.sol: https://github.com/code-423n4/2021-12-yetifinance/blob/main/packages/contracts/contracts/YUSDToken.sol Line 229 Line 230 Line 238 Line 246 Line 254 Line 255

StabilityPool.sol: https://github.com/code-423n4/2021-12-yetifinance/blob/main/packages/contracts/contracts/YUSDToken.sol Line 569 Line 611 Line 656

TroveManager.sol: https://github.com/code-423n4/2021-12-yetifinance/blob/main/packages/contracts/contracts/YUSDToken.sol Line 502 Line 571 Line 640 Line 646 Line 672 Line 743

Tools Used

Manual analysis

Recommended Mitigation Steps

Use require() instead of assert() unless there is a very specific reason that the behavior of assert() is favored. Alternatively, use Solidity 0.8.0 which returns gas with assert()

[kingyetifinance]

@LilYeti: Should be gas severity and duplicate with #51

[alcueca]

Duplicate #157