Sublime contest - defsec's results

Handle

defsec

Vulnerability details

Impact

Setter functions for critical contract parameters accessible only by privileged roles e.g. admin should consider adding timelocks (along with emitted events) so that users and other privileged roles can detect upcoming changes and have the time to react to them.

Changes to whitelists, oracle addresses and migrator address may have a financial or trust impact on users who should be given an opportunity to react to them by exiting/engaging without being surprised when such changes are made effective immediately.

See similar Medium-severity finding in ConsenSys's Audit of 1inch Liquidity Protocol (https://consensys.net/diligence/audits/2020/12/1inch-liquidity-protocol/#unpredictable-behavior-for-users-due-to-admin-front-running-or-general-bad-timing)

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2021-12-sublime/blob/main/contracts/PriceOracle.sol#L189

https://github.com/code-423n4/2021-12-sublime/blob/main/contracts/PriceOracle.sol#L203

2. The functions are responsible for the price oracles. Therefore, If the price oracle is set to wrong. All price feeds will be affected by this.

Tools Used

None

Recommended Mitigation Steps

Consider adding timelocks to such contracts with critical setter functions.

Handle

defsec

Vulnerability details

Impact

Using newer compiler versions and the optimizer gives gas optimizations and additional safety checks are available for free.

The advantages of versions 0.8.* over <0.8.0 are:

• Safemath by default from 0.8.0 (can be more gas efficient than library based safemath.)
• Low level inliner : from 0.8.2, leads to cheaper runtime gas. Especially relevant when the contract has small functions. For example, OpenZeppelin libraries typically have a lot of small helper functions and if they are not inlined, they cost an additional 20 to 40 gas because of 2 extra jump instructions and additional stack operations needed for function calls.
• Optimizer improvements in packed structs: Before 0.8.3, storing packed structs, in some cases used an additional storage read operation. After EIP-2929, if the slot was already cold, this means unnecessary stack operations and extra deploy time costs. However, if the slot was already warm, this means additional cost of 100 gas alongside the same unnecessary stack operations and extra deploy time costs.
• Custom errors from 0.8.4, leads to cheaper deploy time cost and run time cost. Note: the run time cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.

Proof of Concept

1. The contest repository contracts contain different pragma versions. The contracts pragma version can be updated to 0.8.4 for the gas optimization.

• Affected Contracts

"All Contracts"

Tools Used

None

Recommended Mitigation Steps

Consider to upgrade pragma to at least 0.8.4.

Handle

defsec

Vulnerability details

Impact

The use of _msgSender() when there is no implementation of a meta transaction mechanism that uses it, such as EIP-2771, very slightly increases gas consumption.

Proof of Concept

_msgSender() is utilized three times where msg.sender could have been used in the following function.

""" https://github.com/code-423n4/2021-12-sublime/blob/e688bd6cd3df7fefa3be092529b4e2d013219625/contracts/yield/YearnYield.sol#L42

https://github.com/code-423n4/2021-12-sublime/blob/e688bd6cd3df7fefa3be092529b4e2d013219625/contracts/yield/CompoundYield.sol#L43

https://github.com/code-423n4/2021-12-sublime/blob/e688bd6cd3df7fefa3be092529b4e2d013219625/contracts/yield/AaveYield.sol#L71

https://github.com/code-423n4/2021-12-sublime/blob/e688bd6cd3df7fefa3be092529b4e2d013219625/contracts/yield/NoYield.sol#L31

"""

Tools Used

None

Recommended Mitigation Steps

Replace _msgSender() with msg.sender if there is no mechanism to support meta-transactions like EIP-2771 implemented.