Skip to content

Latest commit

 

History

History
466 lines (333 loc) · 36.5 KB

TestingReports.md

File metadata and controls

466 lines (333 loc) · 36.5 KB

Findings

[high]

ETH Sent to a Potentially Arbitrary Address

Within L1Sender.sol, there are multiple instance of ETH being sent to an arbitrary address. For instance:

  • To the ILayerZeroEndpoint variable.
  • To the config variable.

Consider using a whitelist of addresses to send ETH to, or ensure you send ETH to the desired address.

[note]

Possible Presence of Bugs in Target Solidity Version

Please ensure that the codebase is not impacted by any of the enumerated Solidity bugs.

[ethtrust]

Multiple Possible Violations of 'No Conflicting Inheritance' EEA Ethtrust Security Level [S] Requirement

Throughout the codebase there are multiple possible violations. For instance:

Consider solving conflicting inheritance issues.

[note]

Constants Not Using UPPER_CASE Format

In L2TokenReceiver.sol there are constants not using UPPER_CASE format. For instance:

  • The router constant declared on line 14
  • The nonfungiblePositionManager constant declared on line 15

According to the Solidity Style Guide, constants should be named with all capital letters with underscores separating words. For better readability, consider following this convention.

[note]

Variables Are Initialized to Their Default Values

Within Distribution.sol, some variables are being initialized to their default values. For instance:

  • The i variable.
  • The i variable.

To avoid wasting gas uselessly, consider not initializing the variables to the same value that they have by default when they are being declared.

[low]

Floating Pragma

Throughout the codebase there are multiple floating pragma directives. For instance:

Use fixed pragma version.

[note]

Function Visibility Overly Permissive

Throughout the codebase, instances of functions with unnecessarily permissive visibility were found. For instance:

To better convey the intended use of functions and to potentially realize some additional gas savings, consider changing a function's visibility to be only as permissive as required.

[note]

Inconsistent Use of Named Returns

Contract L2TokenReceiver contract has inconsistent usage of named returns in its functions.

Consider naming all returns of all functions.

[high]

Incorrect Interface Implementations

Throughout the codebase, multiple contracts do not implement some interfaces well. For instance: The L2TokenReceiver contract in L2TokenReceiver.sol does not implement the IERC165 Interface well. Check the conformity checker output:

Conformity Checker IERC165:

Functions:

[❌] supportsInterface(bytes4) is implemented incorrectly.
	[✅] supportsInterface(bytes4) signature is correct.
	[❌] PUBLIC visibility is incorrect, should be EXTERNAL.
	[✅] VIEW mutability is correct.
	[✅] [bool] return type is correct.

The MOR contract in MOR.sol does not implement the IERC165 Interface well. Check the conformity checker output:

Conformity Checker IERC165:

Functions:

[❌] supportsInterface(bytes4) is implemented incorrectly.
	[✅] supportsInterface(bytes4) signature is correct.
	[❌] PUBLIC visibility is incorrect, should be EXTERNAL.
	[✅] VIEW mutability is correct.
	[✅] [bool] return type is correct.

To avoid unexpected behavior, ensure that all contracts correctly implement their public interfaces.

[note]

In for Loop Headers Use Prefix Increment Operator ++i to Save Gas

In Distribution.sol this optimization could be applied. For instance:

Consider using the prefix increment operator ++i instead of the post-increment operator i++ in order to save gas. This optimization skips storing the value before the incremental operation, as the return value of the expression is ignored.

[note]

Lack of Security Contact

Throughout the codebase, there are contracts that do not have a security contact. For instance:

Consider adding a NatSpec comment containing a security contact on top of the contracts definition. Using the @custom:security-contact convention is recommended as it has been adopted by the OpenZeppelin Wizard and the ethereum-lists.

[low]

Missing Docstrings

Throughout the codebase there are several parts that do not have docstrings. For instance:

Consider thoroughly documenting all functions (and their parameters) that are part of any contract's public API. Functions implementing sensitive functionality, even if not public, should be clearly documented as well. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

[note]

Possible Precision Loss Due to Division Before Multiplication in Multiples Operations

In LinearDistributionIntervalDecrease.sol, there are multiple multiplications that can lead to precision loss. For instance:

  • The multiplication *.
  • The multiplication *.
  • The multiplication *.
  • The multiplication *.
  • The multiplication *.
  • The multiplication *.
  • The multiplication *.

Performing multiplication before division is generally better to avoid loss of precision, consider ordering multiplication before division.

[low]

Require Statement With Multiple Conditions

Within Distribution.sol the require( amount_ > 0 && (newDeposited_ >= pool.minimalStake || newDeposited_ == 0), "DS: invalid withdraw amount" ) requires multiple conditions to be satisfied.

To simplify the codebase and raise the most helpful error messages for failing require statements, consider having a single require statement per condition.

[low]

Variable Names Too Similar

Throughout the codebase, there are multiple variables with similar names. For instance:

Consider to rename the variables using clear and descriptive variable names and adhering to a naming convention.

[note]

Functions Are Updating the State Without Event Emissions

Throughout the codebase, instances of functions that are updating the state without an event emission were found: For instance:

Consider emitting events whenever there are state changes to make the codebase less error prone and improve its readability.

[High]

Dangerous Use of transferfrom

The use of ERC20's transferFrom on line 195 in Distribution.sol does not pass msg.sender as a from parameter.

Consider using msg.sender as from parameter in ERC20's transferFrom calls.

[ethtrust]

External Call Failure Check Is Missing

The call ILayerZeroEndpoint(config.gateway).send{value: msg.value}( config.receiverChainId, // communicator LayerZero chainId receiverAndSenderAddresses_, // send to this address to the communicator payload_, // bytes payload payable(refundTo_), // refund address address(0x0), // future parameter bytes("") // adapterParams (see "Advanced Features") ) within the contract L1Sender in L1Sender.sol has missing failure check.

undefined

[note]

Uninitialized Local Variables

Throughout the codebase, some local variables are not initialized. For instance:

To improve the overall clarity, intent, and readability of the codebase, consider initializing all local variables.

[note]

Unused Function Arguments

Within the Distribution contract, the None argument for the _authorizeUpgrade function is unused.

To improve the overall clarity, intentionality, and readability of the codebase, consider removing any unused function parameters.

[note]

Unused Function With Internal or Private Visibility

In Distribution.sol the _authorizeUpgrade function is unused.

To improve the overall clarity, intentionality, and readability of the codebase, consider using or removing any currently unused functions.

[note]

Unused State Variable

Within the Distribution contract, the isNotUpgradeable state variable is unused.

To improve the overall clarity, intentionality, and readability of the codebase, consider removing any unused state variables.

[Low]

Void Constructor Call

The constructor of contract MOR calls void constructor of ERC20("MOR", "MOR").

Remove the constructor call.

[high]

Reentrant Codes Identified on a Functions

Throughout the codebase there are contracts containing code that does not protect against reentrancy. For instance:

Consider following the checks-effects-interactions pattern whenever possible, or otherwise protecting code from potential reentrancy.

[note]

Fuzzing Testing Opportunities

undefined

[note]

Use Custom Errors

Throughout the codebase, instances of revert and/or require messages were found. For instance:

For conciseness and gas savings, consider replacing require and revert messages with custom errors.

[note]

Function Might Have an Incorrect ABI Decoding

The lzReceive function in L2MessageReceiver.sol might have an incorrect ABI decoding.

Consider carefully checking the abi decodings to prevent decoding in unexpected ways.

[note]

Incremental Update Is Not Wrapped in an Unchecked Block

Consider wrapping the incremental update into an unchecked block to save the gas required to check against overflows.

[note]

Assembly

undefined