Inverse Finance contest - sakshamguruji's results

USE OF tx.origin

The use of tx.origin can be manipulated by fishing the origin address by executing a transaction on attacker's behalf therefore an attacker can pretend to be the originator of the contract and execute the function protected by something like require(msgSender = tx.origin). Read more about it here https://solidity-by-example.org/hacks/phishing-with-tx-origin/.

Affected Code: https://github.com/code-423n4/2022-10-inverse/blob/main/src/BorrowController.sol#L47

Remediation: Use something like require(msg.sender == owner) and in the constructor set owner = msg.sender so that msg.sender is the owner of the contract or the who deployed the contract.

CHECK IF borrowController !=0

In the function https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L389 it checks borrowController != IBorrowController(address(0)) , but if it is zero then it would bypass that check and execute the function without the check if the borrowController is allowed to borrow or not. Simply add a check borrowController !=0

EVENT IS MISSING INDEXED FIELDS

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it’s not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

Affected Code:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L140 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L141 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L381 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L382 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L614-L620 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L147

USE A MODIFIER INSTEAD OF USING THE SAME REQUIRE STATEMENT AGAIN AND AGAIN

Instead of using the same require statements again and again , it should be considered to put those statement in a modifier and then use those modifiers in functions where needed.

Affected code:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L49 This require statement should be kept in a modifier and that modifier can be used in function https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L48 , https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L57 , https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L66 Similarly , use a onlyChair modifier for the functions that require msg.sender is chair.

LACK OF ZERO ADDRESS CHECKS

There are multiple functions where they lack zero address checks , if set to zero address these functions can set the addresses to useless addresses and make them useless.

Affected code:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/SimpleERC20Escrow.sol#L36 Check if the recipient is not a zero address , if it is funds may be transferred to a zero address and be lost forever

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L57 Add a 0 address check here https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L53 to avoid setting the pendingOperator to a 0 address.

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L81 Here we set the minter , add a zero address check here so that we dont add invalid address(s) to this mapping.

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L99 Here we add a market to the markets mapping , add a zero address check here so that we dont add invalid address(s) to this mapping and populate the mapping with garbage values.

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L158 Add a zero address check in the approve function to avoid mistakenly approving zero address .

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L66 Avoid mistakenly setting the chair address to 0 address.

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L300 Might save gas to check 0 address beforehand .(Further checks won't be executed)

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L313

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L325

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L118 Avoid setting the oracle to a 0 address

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L124 Avoid setting the borrowController to a 0 address

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L136 Avoid setting the lender to a zero address.

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L142 Avoid setting the pauseGuardian to zero address

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L591 check if the user to liquidate is not the zero address.

SPELLING MISTAKES IN CODE

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L45 Instead of fed contact should be fed contract

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L87 Change allowe to allowed

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L128 Change th to the

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L146 Change mus to must

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L589 Change Th to The

CHANGE FUNCTION NAMES TO AVOID CONFUSION

Consider changing the names of these functions to avoid confusion as there are more function with similar name -

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L278 Change the name to something like depositOnBehalf as there is already a function with name deposit.

FUNCTION PARAMETERS SHOULD HAVE A UNDERSCORE

It is a convention to declare function parameters with an underscore. Instances of functions with parameters without an underscore -

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/SimpleERC20Escrow.sol#L36

https://github.com/code-423n4/2022-10-inverse/blob/main/src/BorrowController.sol#L32 https://github.com/code-423n4/2022-10-inverse/blob/main/src/BorrowController.sol#L38 https://github.com/code-423n4/2022-10-inverse/blob/main/src/BorrowController.sol#L46 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L86 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L103 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L120 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L131 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L120 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L133 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L146 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L158 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L170 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L188 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L215 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L284 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L300 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L313 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L325 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L340 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L349 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L359 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L372 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L53 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L61 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L78 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L112 https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/INVEscrow.sol#L59 https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/INVEscrow.sol#L90 https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/GovTokenEscrow.sol#L43 https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/GovTokenEscrow.sol#L66 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L203 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L226 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L245 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L258 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L267 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L278 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L292 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L312 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L323 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L334 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L344 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L353 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L370 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L389 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L408 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L422 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L460 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L472 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L486 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L531 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L546 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L559 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L578 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L591

INTERNAL FUNCTIONS SHOULD BEGIN WITH AN UNDERSCORE

It is a naming convention to name the internal functions by having a underscore at the start(or at the end) . Change the names from e.g. functionName to _functionName. Instances of this issue:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L101 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L226 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L245 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L323 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L344 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L353 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L389 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L460

FLOATING PRAGMA USED THROUGHOUT THE CODEBASE

Throughout the codebase floating pragma version has been used , to be on a safer side all the development should be done on a fixed pragma/solidity version as there may be a vulnerability in the later versions within that pragma which may be abused in the future.

Check If A Minter Already Exists

On the function here https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L195 it adds a new minter , there should be a require statement like this require(minters[_minter] == false) to check if the minter is already added this would save gas as the function will not execute fully(setting the mapping yo true and then emitting an event)

Similarly , add a require that require(minters[_minter] == true) here in this function https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L90

and Similarly check if the market already exists here https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L90

Change > to >= and < to <= To Save Gas

On line https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L123 instead of doing if(dueTokensAccrued[user] + accrued > balances[user]) return 0 do if(dueTokensAccrued[user] + accrued >= balances[user]) return 0because ifdueTokensAccrued[user] + accrued = balances[user]then the statement belowbalances[user] - dueTokensAccrued[user] - accrued` would return zero , so changing to >= would make us avoid the last check .

Similarly , https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L133 here in this function instead of doing dueTokensAccrued[user] + accrued < balances[user] do this dueTokensAccrued[user] + accrued <= balances[user]

X += Y costs 22 more gas than X = X + Y

This can mean a lot of gas wasted in a function call when the computation is repeated n times (loops)

Recommended Mitigation

use X = X + Y instead of X += Y (same with -).

Affected Code:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L91 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L92 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L110 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L111 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L172 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L174 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L196 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L198 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L288 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L289 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L304 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L316 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L332 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L360 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L362 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L374 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L376 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L395 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L397 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L534 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L535 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L565 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L568 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L598 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L599 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L600

MULTIPLE ADDRESS/ID MAPPINGS CAN BE COMBINED INTO A SINGLE MAPPING OF AN ADDRESS/ID TO A STRUCT, WHERE APPROPRIATE

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key’s keccak256 hash (Gkeccak256 - 30 gas) and that calculation’s associated stack operations.

Affected Code:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L19-L28 (combine all address -> something mappings to one address -> struct mapping) https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L57-L59 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol#L25-L26

REDUCE THE SIZE OF ERROR MESSAGES (LONG REVERT STRINGS)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Affected Code:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L63 https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L326 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L76 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L214

SPLITTING REQUIRE() STATEMENTS THAT USE && SAVES GAS

With enough runtime calls, the change ends up being cheaper i.e. splitting require statements using &&.

Affected Code:

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol#L249 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L448 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L512 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L75 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L162 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L173 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L184 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol#L195