Links

Omniscia Audit Report

Access the full report here
Here is a breakdown of some key issues or misbehaviours identified by Omniscia and the fixes implemented wherever it was required. During the audit, Omniscia filtered and validated a total of 6 findings utilizing static analysis tools as well as identified a total of 18 findings during the manual review of the codebase.

Issue ID and Fixes:

MAN-01S: Arbitrary Code Execution

Auditor Severity Rating: Medium
Description: The linked statement performs arbitrary code execution that could even lead to the contract deleting itself
Status: Not Addressed
Alleviation: We considered this exhibit but opted not to apply remediation for it in the current iteration of the codebase

MAN-02S: Inexistent Validation of Input Address

Auditor Severity Rating: Minor
Description: The linked function accepts an address input argument yet does not properly sanitize it
Status: Nullified
Alleviation: The setStaking function has been entirely omitted from the codebase thereby nullifying this exhibit

MAN-03S: Inefficient Usage of EnumerableSet

Auditor Severity Rating: Informational
Description: The add and remove instructions of the EnumerableSet yield a bool variable signalling whether the operation was executed successfully which can be utilized in place of the contains checks.
Status: Fixed
Alleviation: All linked instances have been refactored to properly utilize the returned bool of the addition and removal operations on the EnumerableSet.

POO-01S: Variable Shadowing

Auditor Severity Rating: Informational
Description: The linked variables shadow existing declarations in parent contracts
Status: Fixed
Alleviation: The variable shadowing has been corrected by renaming the linked variables as advised

STA-01S: Inefficient Usage of EnumerableSet

Auditor Severity Rating: Informational
Description: The add and remove instructions of the EnumerableSet yield a bool variable signalling whether the operation was executed successfully which can be utilized in place of the contains checks.
Status: Nullified
Alleviation: The file is no longer present in the codebase thereby nullifying this exhibit

STA-02S: Redundant Zero Value Writes

Auditor Severity Rating: Informational
Description: The linked zero value writes are redundant as that's the default value of the variable they are assigned to
Status: Nullified
Alleviation: The file is no longer present in the codebase thereby nullifying this exhibit

MAN-01M: Mismatch of Access Control

Auditor Severity Rating: Medium
Description: The setBalance function is meant to be invoked only by the administrator of the system yet the updateBalance function it internally invokes imposes a different set of access controls.
Status: Partially Fixed
Alleviation: The code was refactored to instead check if the caller is an administrator and the conditional check for the staking address was omitted. Omniscia advises the OliveDAO team to provide additional justification as to why the access control differs between those functions in this way.

MAN-02M: Improper Utilization of Duration

Auditor Severity Rating: Minor
Description: The variable cycleDuration is meant to represent the duration of each cycle, however, based on the bound checking performed by the contract the actual cycle duration is always equal to cycleDuration + 1 as a value of 0 would still require a single block to pass between cycles.
Status: Partially Fixed
Alleviation: The sanitizations advised have been introduced and the comparison was made inclusive but only for the first linked instance thereby partially alleviating this exhibit.

MAN-03M: Inexistent Rollover State Validation

Auditor Severity Rating: Minor
Description: The linked functions should validate the value of rolloverStarted in the form of a require check however they do not, rendering the variable useless.
Status: Fixed
Alleviation: A require check has been introduced in both functions validating the value of rolloverStarted.

MAN-04M: Inexplicable Re-Invocation Capability

Auditor Severity Rating: Minor
Description: The linked functions adjust sensitive contract variables and can be executed an arbitrary number of times.
Status: Fixed
Alleviation: The setStaking function has been omitted from the codebase whilst the setVoting function has been made as invoke-able only once thereby alleviating this exhibit.

POO-01M: Potentially Improper Balance Relay

Auditor Severity Rating: Minor
Description: The linked statements relay the balance adjustments independently, emitting misleading events in case of a transfer to self.
Status: Fixed
Alleviation: Transfers to self are now properly prohibited by a corresponding require check-in both functions.

RHH-01M: Inexistent Validation of Existing Entry

Auditor Severity Rating: Minor
Description: The setCycleHashes permits the owner to override previously set hashes.
Status: Fixed
Alleviation: A require check was introduced ensuring that the cycle hash hasn't already been set thereby alleviating this exhibit.

VOE-01M: System Voting Power Desynchronization

Auditor Severity Rating: Major
Description: The setVoteMultipliers function will adjust the vote multiplier but will not update each user's existing vote.
Status: Nullified
Alleviation: The relevant function is no longer present in the codebase rendering this exhibit null.

VOE-02M: Inexistent Access Control of Event Emission

Auditor Severity Rating: Medium
Description: The updateBalance function is meant to emit a unique WithdrawalRequestApplied event that off-chain processes are meant to react to yet it does not impose any access control.
Status: Fixed
Alleviation: The function now properly applies access control via the onlyStaking modifier alleviating this exhibit.

MAN-01C: Inefficient Re-Entrancy Guard

Auditor Severity Rating: Informational
Description: The linked re-entrancy guard is custom made and utilizes a bool to signal its status.
Status: Fixed
Alleviation: The OpenZeppelin ReentrancyGuard implementation is now properly utilized in the codebase.

MAN-02C: Inefficient Utilization of Length

Auditor Severity Rating: Informational
Description: The linked statements perform a storage lookup on the value of length on each iteration of the loops.
Status: Fixed
Alleviation: The length member is now cached to a local variable in both linked instances alleviating this exhibit.

MAN-03C: Redundant Usage of SafeMath

Auditor Severity Rating: Informational
Description: The project's code is compiled with a compiler version beyond 0.8.X which has built-in safe arithmetics toggled on by default.
Status: Fixed
Alleviation: The redundant usage of SafeMath has been safely omitted from the codebase.

POO-01C: Inefficient Re-Entrancy Guard

Auditor Severity Rating: Informational
Description: The linked re-entrancy guard is custom made and utilizes a bool to signal its status.
Status: Fixed
Alleviation: The OpenZeppelin ReentrancyGuard implementation is now properly utilized in the codebase.

POO-02C: Redundant Usage of SafeMath

Auditor Severity Rating: Informational
Description: The project's code is compiled with a compiler version beyond 0.8.X which has built-in safe arithmetics toggled on by default.
Status: Fixed
Alleviation: The redundant usage of SafeMath has been safely omitted from the codebase.

REW-01C: Redundant Usage of SafeMath

Auditor Severity Rating: Informational
Description: The project's code is compiled with a compiler version beyond 0.8.X which has built-in safe arithmetics toggled on by default.
Status: Fixed
Alleviation: The redundant usage of SafeMath has been safely omitted from the codebase.

RHH-01C: Ineffectual constructor Implementation

Auditor Severity Rating: Informational
Description: The linked constructor is redundant as the default value of latestCycleIndex is 0.
Status: Fixed
Alleviation: The redundant assignment has been safely omitted from the codebase. The redundant assignment has been safely omitted from the codebase.

RHH-02C: Redundant Usage of SafeMath

Auditor Severity Rating: Informational
Description: The project's code is compiled with a compiler version beyond 0.8.X which has built-in safe arithmetics toggled on by default.
Status: Fixed
Alleviation: The redundant usage of SafeMath has been safely omitted from the codebase.

STA-01C: Redundant Usage of SafeMath

Auditor Severity Rating: Informational
Description: The project's code is compiled with a compiler version beyond 0.8.X which has built-in safe arithmetics toggled on by default.
Status: Nullified
Alleviation: The file is no longer present in the codebase thereby nullifying this exhibit.

VOE-01C: Redundant Variable Visibility

Auditor Severity Rating: Informational
Description: The linked variable is meant to be an internally accessible constant yet is declared as public.
Status: Fixed
Alleviation: The variable has been properly set as private alleviating this exhibit.

Post-Audit Conclusion

The OliveDAO team provided remediation for the most important exhibits outlined in the report and partially alleviated some which we advise them to re-visit, such as MAN-02M.