Messier - Audit
CertiK reviewed MESSIER (Messier - Audit) on 23 May 2023: 28 findings, 0 critical; 25 marked fixed, 3 acknowledged.
Report hosted on skynet.certik.com.
Trustblock does not review code.
Findings28
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 0 |
| High | 0 | 1 | 2 |
| Medium | 0 | 0 | 7 |
| Low | 0 | 2 | 16 |
HighA user could sign a proposal for multiple timesFixed
In the `signOnProposal()` function, users can only sign a proposal once due to the `notSigner(proposalIndex)` modifier, which checks whether `_storeSigner[msg.sender][_proposalIndex] == false`. Therefore, the `_storeSigner[msg.sender][_proposalIndex] = true` code should be used for each signing process. However, the contract uses `_storeSigner[msg.sender][signersNum] = true;`, which allows a user to sign a proposal multiple times.
HighCentralization Related Risks in `ProposalCheckOutFixed
In the contract `Dao` the role `ORACLE` has authority over the functions below. - function `ProposalCheckOut`: to execute a proposal according to the `ProposalType`. Any compromise to the `ORACLE` account may allow the hacker to take advantage of this authority.
MediumLack of Quorum RestrictionFixed
In the function `CheckOutVote()`, the voting result is only based on the number of YES and NO votes without considering the total amount of votes, namely the quorum. As a result, the vote could be passed with only one YES vote.
MediumLogical issue in function `changeState()`Fixed
The logic in the `changeState` function is incomplete and incorrect. Specifically, the `VoteOnProposal` function can only be invoked in the `QUEUED` state, but there is no mechanism to transition to or from that state: 1. The `CREATED` state can only be changed to `FAILED`. 2. There is currently no mechanism in place to transition from the `QUEUED` state to the `VOTING` state. 3. It is not possible to change the state to any value other than `CREATED` or `VOTING` once the proposal has entered either of these states. ```solidity = 703 function changeState(State _state,uint _proposalIndex) public OnlyOracle returns(bool){ if(proposals[_proposalIndex].activeState == State.CREATED){ if(_state == State.FAILED){ proposals[_proposalIndex].activeState = _state; }else{ revert("You can not change whatever you want"); } }else if(proposals[_proposalIndex].activeState == State.VOTING){ (uint YES,uint NO,State s) = CheckOutVote(proposalIndex); if(NO>YES){ proposals[_proposalIndex].activeState =State.FAILED; }else{ proposals[_proposalIndex].activeState =State.READY; } } ```
MediumLogical issue on the voting systemFixed
The `oracle` role has the ability to directly execute a proposal by calling `OracleNext()` regardless of the result of the voting process, which has the potential to render the entire voting system invalid.
MediumLogical issue when processing the `queue_Ids` inFixed
The following code snippets in the `ProposalCheckOut()` function is incorrect: ```solidity if (queue_Ids[_Index] == _idQ) { queue_Ids[_Index] = queue_Ids[_Index]; queue_Ids.pop(); ``` , for the `queue_Ids[_Index] = queue_Ids[_Index];` is redundant, it should be `queue_Ids[_Index] = queue_Ids[queue_Ids.length - 1];` to implement the design.
MediumMissing Receive FunctionFixed
There is no `receive()` or `fallback()` function in the Dao.sol, so this contract cannot receive ETH.
MediumThe `CloseProposal()` will not workFixed
The `CloseProposal()` function cannot be called because a vote can only be in the `State.CREATED` or `State.EXECUTED` state according to the code. Additionally, changing the state to `State.FAILED` has no effect as there is no state check on `VoteOnProposal()`, meaning that the voting process cannot be stopped.
MediumWrong state checkFixed
According to the logic of the function `ProposalNext()`, the state check in `ProposalCheckOut()`, which is `if(proposal.activeState ==State.QUEUED)`, should be modified as `if(proposal.activeState ==State.RUNNING)`, or the `ProposalNext()` would be reverted.
Low`Initializable` is Not ImportedFixed
The `Initializable` contract is inherited but not imported.
Show 10 more
LowLack of check on `proposal.date`Fixed
If the proposal has a time limit, the contract should verify that the current time is within the voting period before calling `VoteOnProposal()`.
LowMissing Emit EventsFixed
There should always be events emitted in the sensitive functions that are controlled by centralization roles.
LowMissing Error MessagesFixed
The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.
LowMissing Zero Address ValidationFixed
Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=160 ORACLE = _oracle; ``` - `_oracle` is not zero-checked before being used. --- ```solidity=161 _M87 = _m87; ``` - `_m87` is not zero-checked before being used. --- ```solidity=188 _supernova = _fb; ``` - `_fb` is not zero-checked before being used.
LowPotential Sandwich AttacksFixed
A sandwich attack may happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (executing before the target) a transaction to purchase one of the assets and make profits by back running (executing after the target) a transaction to sell the asset. The following function is called without setting restrictions on slippage or minimum output amount, so transactions triggering the function are vulnerable to sandwich attacks, especially when the input amount is large: - `IUniswapV2Router(UNISWAP_V2_ROUTER).swapExactTokensForETHSupportingFeeOnTransferTokens()` in `swapTokensForEth()`
LowRedundant `Bridge` modifierFixed
The `Bridge` modifier verifies whether the `_hsh` parameter is equal to the `_Hash` value. However, since the `_hsh` is always passed in as the `_Hash` parameter and is not set by the function caller, the modifier becomes redundant.
LowRedundant `CLASSIC_MAX` and `CLASSIC_MIN`Fixed
The `CLASSIC_MAX` and `CLASSIC_MIN` is not used anywhere in the contract, hence they are redundant.
LowRedundant `ProposalCheckOut()`Fixed
The private function `ProposalCheckOut()`, which verifies and executes the result of a proposal, is redundant as it is not utilized anywhere in the contract.
LowRedundant codeFixed
In consideration of the functionality of the `changeState()` function and its capacity to modify a proposal's state at will, it appears unnecessary to use the `CloseProposal()` function to change the proposal state to `FAILED`. Additionally, the `failedChecker` modifier has also been considered redundant since every function that utilizes this modifier already incorporates an `atState` check.
LowToo Many DigitsFixed
Literals with many digits are difficult to read and review. ```solidity=164 CLASSIC_DEF = 8700000000000000000; //wei 8.7 ``` --- ```solidity=165 DOUBLE_DEF = 17400000000000000000; //wei 17.4 ``` --- ```solidity=166 CLASSIC_MIN = 870000000000000000; //wei 0.87 ``` --- ```solidity=167 DOUBLE_MIN = 8700000000000000000; //wei 8.7 ``` --- ```solidity=168 CLASSIC_MAX = 87000000000000000000; //wei 8.7 ``` --- ```solidity=169 DOUBLE_MAX = 26100000000000000000; //wei 26.1 ``` --- ```solidity=170 FULLCYCLE_MAX = 87000000000000000000; //wei 87 ETH ```
Show 5 more
LowTypoFixed
The `persentage` should be `percentage`.
LowUnchecked ERC-20 `transfer()`/`transferFrom()` CaFixed
The return value of the transfer()/transferFrom() call is not checked. ```solidity=764 IERC20Upgradeable(_token).transfer( address(_supernova), getfromtoken ); ``` --- ```solidity=803 IERC20Upgradeable(_token).transfer(address(0xdead), getfromtoken); ```
LowUnlocked Compiler VersionFixed
The contract cited has an unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to differing compiler version numbers. This can lead to ambiguity when debugging, as compiler specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.
LowUnused private functionFixed
The function `swapTokenForToken` is private but never used in the contract, hence it is redundant.
LowUnused Return VariableFixed
```solidity=390 returns (bool) ``` - The function `ProposalCheckOut` implicitly returns a default value. --- ```solidity=446 returns (bool) ``` - The function `sendToExecution` implicitly returns a default value. --- ```solidity=505 returns (bool) ``` - The function `ProposalNext` implicitly returns a default value. --- ```solidity=713 ) internal Bridge(_hsh) returns (bool) { ``` - The function `execute_oneProposal` implicitly returns a default value. --- ```solidity=791 ) internal Bridge(_hsh) returns (bool) { ``` - The function `execute_Cycle` implicitly returns a default value.
Contracts in scope2
| Address | Chain | Source code |
|---|---|---|
| 0xe0cf9c69894e2cca7221a5d1db2f867ece64b08c | Chain: Ethereum | Source code: Verified source |
| 0x2b653f343e11ded7daef015aa3034594c3194dd4 | Chain: Ethereum | Source code: Verified source |
Part of MESSIER’s security record
MESSIERSecurity firm
CertiK1,044 projects reviewed · 1,375 reports · latest May 2025
See CertiK’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us