1inch Mooniswap v2

CertiK16 Dec 2020Files named in the report

CertiK reviewed 1inch Network (1inch Mooniswap v2) on 16 Dec 2020: 36 findings, 0 critical; 27 marked fixed, 9 open.

No open critical findings

Open the report Open 1inch Network’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings36

Statuses are the security firm’s own, from its report.

Severity	Open	Fixed
Critical	0	0
High	0	0
Medium	1	2
Low	8	25

MediumIncorrect Stake Notification ImplementationFixed

The `notifyFor` function is meant to be used to allow any address to force a status update of another `address` arbitrarily to ensure the stakes states are kept in sync. However, the second argument passed to the `_notifyFor` function is the balance of the `msg.sender` instead of the account to-be-notified, causing an incorrect balance to be reported for it and potentially for as many accounts as a particular `msg.sender` wants.

MediumInexistent Input Sanitization of Mooniswap AddresFixed

The linked code blocks link to function implementations within `ReferralFeeReceiver` that do not conduct any sanitization on the input `mooniswap` variable and invoke functions on it.

Low`require` ConsistencyFixed

The project imposes `require` checks on multiple functions, like L255 of `_doTransfers`, to ensure that zero value swaps etc. are prohibited. The `withdrawFor` function fails to impose such a check as it merely ensures that the `value` is greater-than-or-equal to `minReturns[i]` which itself could be `0`.

Low`SafeMath` RedundancyFixed

The linked mathematical operations utilize their `SafeMath` counterpart implementation whereas it is completely reudndant for the operations of L31 and L34 as well as potentially redundant for the operation of L30.

Low`SafeMath` RedundancyFixed

The linked statement conducts a `SafeMath` subtraction operation on the `_totalSupply` variable based on the amount burned from the `_balances` mapping. As the contract level variables of the contract are `private`, they cannot be altered by derivative implementations and as such, the `_totalSupply` is kept in perfect sync with the amount of total balance held in the `_balances` mapping. As a result, if a `SafeMath` subtraction operation succeeds on the _balances mapping it is guaranteed to succeed in the `_totalSupply` variable as well, rendering its use redundant gaswise.

Low`SafeMath` RedundancyFixed

The linked statements all conduct `SafeMath` operations whilst the safety of their operations is guaranteed by the system either via `if-else` clauses or constant variable utilizations

Low`SafeMath` RedundancyFixed

The linked statements both contain redundant operations as they are nested within an `if-else` block that guarantees their validity

Low`SafeMath` RedundancyFixed

The linked statements conduct `SafeMath` operations when their safety may be guaranteed by the system for the first and last linked statements and is fully guaranteed for the middle statement.

LowDisproportionate Initial MintingFixed

The initial minting process of a Mooniswap appears to be disproportionate in the sense that it does not rely on the initial deposit, meaning that a user can simply deposit only a single unit and acquire a mint equal to `_BASE_SUPPLY.mul(99)` as only a `Math.max` operation is used on L145.

LowDocumentation ConsistencyFixed

The linked documentation block is meant to explain the calculations carried out in `_getReturn` , however the variable naming conventions of the function block do not conform to what is laid out in the documentation and the documentation itself appears inconsistent with the statements of the function.

Show 10 more

LowEvent IndexingFixed

The `Swapped` event is indexing the source token, however, the destination token remains unindexed

LowIncorrect `to` ConditionalFixed

The linked `_beforeTokenTransfer` hook incorrectly sanitizes burn operations by evaluating whether the `from` instead of the to variable is different than the zero address for the ternary operator used on the balanceTo assignment.

LowIncorrect Implementation / Naming ConventionFixed

The naming convention utilizing `tax` alludes to a tax being imposed on the balances, however L323 assigns the addition instead of the subtraction of the tax from the `srcBalance`.

LowIncorrect len Boundry CheckFixed

The boundary check of the `len` variable is meant to ensure that the `len` variable can fit within a single 8-bit slot by ensuring its within its valid range. However, the upper bound specified is incorrect.

LowIncorrect Transfer EvaluationFixed

The linked code invokes `uniTransferFrom` for an amount equal to `maxAmounts[i]` and consequently assigns that value to `receivedAmounts[i]` even if a token imposes fees on the transaction.

LowInexistent Access ControlFixed

The linked functions are meant to conduct sensitive operations on the contract and utilize its full balance to conduct a swap. Even though the target of a given `swap` operation will always be the `rewards` address specified during construction time, a malicious path can lead to multiple pairs being exchanged artificially increasing volume and diminishing the final output sent to the `rewards` address.

LowMisleading Error MessageFixed

The error message alludes that ETH transfers are completely forbidden to the contract whereas transfers from contracts are allowed.

LowPotential Code RedundancyFixed

The linked line performs an on-chain conversion from the `address` low-level representation to its human-readable hexadecimal representation.

LowProhibition of Ether TransfersFixed

The contract does not permit Ethereum transfers from contracts in contrast to `GovernanceFeeReceiver` .

LowRedundant `isDefault` InvocationFixed

The `if` conditional that precedes those two statements already evaluates whether the vote is the default one, meaning a literal can be passed here instead.

Show 7 more

LowRedundant Use of Dynamic ArraysFixed

The functions of the Mooniswap contract accept statically sized arrays of 2 members whereas the `withdraw` prefixed functions accept dynamically sized arrays whose size is ensured to be `2` within their respective code block.

LowSafeMath RedundancyFixed

The linked mathematical statement conducts a `SafeMath` operation whilst its safety is guaranteed by the preceding `if` clause.

LowSafeMath RedundancyFixed

The linked statement conducts a `SafeMath` subtraction operation on the _totalSupply variable based on the amount burned from the `_balances` mapping. As the contract level variables of the contract are `private` , they cannot be altered by derivative implementations and as such, the `_totalSupply` is kept in perfect sync with the amount of total balance held in the `_balances` mapping. As a result, if a `SafeMath` subtraction operation succeeds on the `_balances` mapping it is guaranteed to succeed in the `_totalSupply` variable as well, rendering its use redundant gaswise.

LowUnfair Proportionate CalculationFixed

The linked code block is meant to collect an epoch's share by calculating the percentage of shares a user has proportionate to the total supply of shares of a given epoch and multiply the resulting percentage with the total inch balance of the epoch.

LowUser-Defined GettersFixed

The linked variables contain user-defined getter functions that are equivalent to their name barring for an underscore ( `_` ) prefix / suffix.

LowVariable TyposFixed

The linked code lines contain a mispelling of the `default` word as `defaul` , leading to mistyped variable names.

LowWhitelist EvaluationFixed

The linked `for` loop ensures that the elements in the `path` are whitelisted, however the first element is assumed to be whitelisted whilst it is not guaranteed so in the `require` checks of the function.