code-423n4 / 2021-12-yetifinance-findings

0 stars 0 forks source link

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

Closed code423n4 closed 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-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L290-L290

assert(vars.compositeDebt > 0);

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/TroveManager.sol#L571-L571

assert(closedStatus != Status.nonExistent && closedStatus != Status.active);

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/YUSDToken.sol#L229-L230

assert(sender != address(0));
assert(recipient != address(0));

Recommendation

Change to require().

kingyetifinance commented 2 years ago

@LilYeti: Duplicate of #51

alcueca commented 2 years ago

Duplicate #157

alcueca commented 2 years ago

Despite the recommendation from the solidity manual, the practical impact of this issue is that of an excessive gas use in some conditions.