MUD

CertiK16 Dec 2022Files named in the report

Audit

CertiK reviewed Metaverse User DAO (MUD) on 16 Dec 2022: 32 findings, 2 critical; 18 marked fixed, 14 open.

2 critical findings, fixed

Open the report Open Metaverse User DAO’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings32

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

Severity	Open	Fixed
Critical	0	2
High	6	0
Medium	1	1
Low	7	15

CriticalDangerous AuthorityFixed

File: miningEscrow.sol According to the codes in #L152, #L153, `freeAmount` token will be transferred from current contract to the `msg.sender`. But current contract will approve allowance to the `msg.sender`. It is very dangerous because the `msg.sender` can withdraw extra tokens from current contract with the allowance. ```solidity=151 if (freeAmount > 0) { token.transfer(msg.sender, freeAmount); token.increaseAllowance(msg.sender, freeAmount); } ```

CriticalMissing Access ControlFixed

File Token.sol According to the logic of function `mainNetMappingFreeze()`, it can be used to freeze token transfer which means token holders can not transfer `MUD` anymore. However, this function is lack of access control so that anyone can call this function to block `MUD` transaction. ```solidity=33 function mainNetMappingFreeze() public { require(!_frozen); _frozen = true; } ```

MediumUsage of `tx.origin`Fixed

Contracts that authorize users using the `tx.origin` variable are typically vulnerable to phishing attacks which can trick users into performing authenticated actions on the vulnerable contract. There are multiple reasons why using `tx.origin` is not safe from the security perspective. Refer to : * [Solidity Documentation on `tx.origin`](https://docs.soliditylang.org/en/latest/security-considerations.html#tx-origin) * <https://github.com/ethereum/solidity/issues/683> ```solidity=64 require(tx.origin == creator, "not contract creator!");//only onwer can call this function ```

LowDuplicated CheckFixed

File: TRC20.sol According to the logic of function `transferFrom()`, token holders can only transfer tokens when the transfer function is not frozen. However, this check is also added to the function `_transfer()`, so the relevant check is duplicated. ```solidity=98 function transferFrom(address sender, address recipient, uint256 amount) public returns (bool) { require(!_frozen); _transfer(sender, recipient, amount); _approve(sender, msg.sender, _allowances[sender][msg.sender].sub(amount)); return (true); } ``` ```solidity=167 function _transfer(address sender, address recipient, uint256 amount) internal { require(!_frozen); require(sender != address(0), "TRC20: transfer from the zero address"); require(recipient != address(0), "TRC20: transfer to the zero address"); _balances[sender] = _balances[sender].sub(amount); _balances[recipient] = _balances[recipient].add(amount); emit Transfer(sender, recipient, amount); } ```

LowInconsistent Comment and CodeFixed

In the `MudABRoundReleaseBank` contract, `dailyRate` is 9259 and &10^7& is used to calculate `dailyReleaseAmount`. According to the comments in line #16 and #58, the numbers above should be 92590 and $10^8$. ```solidity=16 uint256 constant dailyRate = 9259; //0.0009259 daily release rate 0.09259% ``` ```solidity=58 bank[investorAddress].dailyReleaseAmount = amount.mul(dailyRate).div(10000000); //amount * dailyRate / 100000000; ``` Comparing these in `MudAngelRoundReleaseBank`, the mathematical formula should be $ration: y(x) = x / 10^8$ (MUD token precision is $10^6$ and $10^2$ is percent). ```solidity=16 uint256 constant dailyRate = 79365; //0.00079365 angel round daily release percentage 0.079365% ``` ```solidity=57 bank[investorAddress].dailyReleaseAmount = amount.mul(dailyRate).div(100000000); ``` In the `MudMiningPool` contract, just as the comments describes the mining halving `dailyMiningLimit` should be updated every 4 years, in the code it's updated every one year. ```solidity=68 //update mining halving dailyMiningLimit every 4 years if (now > lastHalvingTime + 31536000) { dailyMiningLimit = dailyMiningLimit.div(2); lastHalvingTime = now; } ``` The code and comment mentioned above is not consistent.

LowIncorrect Burn AmountFixed

File: Token.sol The code in #L47 will burn `icoLeftoverAmount` tokens from the `creator` account. However, the value of `icoLeftoverAmount` is already set to zero which means no tokens will be burned. ```solidity=45 if (icoLeftoverAmount > 0) { icoLeftoverAmount = 0; _burn(msg.sender, icoLeftoverAmount); } ```

LowIncorrect Error MessageFixed

File: icoReleaseAngelRound.sol The validation in #L103 is used to check if the interval between current time and last release time is greater than one day. A failed validation means that the current time is less than last release time plus `secPerDay`. ```solidity=103 require(now > bank[msg.sender].lastTime + secPerDay, "now < lastTime"); ``` Moreover, the below error messages are also inaccurate since the last time may be updated when the deposit or release occurred. File: icoReleaseAngelRound.sol ```solidity=80 require(now > bank[addressToCheck].lastTime, "now < last release time"); ``` File: icoReleaseRoundAB.sol ```solidity=80 require(now > bank[addressToCheck].lastTime, "now < depositTime"); ```

LowMissing Emit EventsFixed

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

LowMissing Input ValidationFixed

Due to the following restriction, the `admin` can not release tokens. Also, the zero address can not release tokens. So it is better to add a restriction in the deposit functions `teamMemberDeposit()`, `icoDeposit()` and `dailySettlement()`. File `teamRelease.sol`, `icoReleaseRoundAB.sol` and `icoReleaseAngelRound.sol` ```solidity function releaseToken() public returns (uint256, uint256) { require(msg.sender != admin, "msg.send == admin"); ... } ``` File `miningPool.sol` ```solidity function withdraw() public returns (uint256) { require(msg.sender != admin && msg.sender != miningDappAddress,"admin and dapp not allowed!"); ... } ```

LowPotential Deposit IssueFixed

In the implementation of deposit mechanism, only admin can deposit a specified amount of `MUD` token to respective contracts for investors or team members. The deposit operation would fail if the balance of contract plus the deposit amount exceeds the upper limit defined within contracts. ```solidity require(amount + token.balanceOf(contractorAddr) <= angelRoundLimit, "amount overflow!"); ``` ```solidity require(amount + token.balanceOf(contractorAddr) <= angelRoundLimit, "amount overflow!"); ``` ```solidity require(amount + token.balanceOf(contractorAddr) <= teamMemberBalance, "amount overflow!"); ``` We cannot ensure that nobody could transfer `MUD` token to these contracts directly, if that happens, the current deposit mechanism may not work as expected. Besides, in the `MudMiningPool` contract, if someone transfers MUD token to this contract prior to deposit operation called by admin, it would also fail. ```solidity require(token.balanceOf(address(this)) == 0);//only deposit once after ICO ```

Show 8 more

LowRedundant Code ComponentsFixed

The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.

LowSafeMath Not UsedFixed

Solidity versions below 0.8.0 don't automatically support overflow handling. Most mathematical operations in `MUD` code use safe math, but some do not. For example: #L70 in miningPool.sol use safe math ```solidity dailyMiningLimit = dailyMiningLimit.div(2); ``` #L63 in miningPool.sol do not use safe math ```solidity require(now > lastDailySettlementTime + 86400, "only settle once per day"); //86400 ```

LowUnchecked ERC-20 `transfer()`/`transferFrom()` CaFixed

The return value of the transfer()/transferFrom() call is not checked. ```solidity=114 token.transfer(msg.sender, freeAmount); ``` --- ```solidity=113 token.transfer(msg.sender, freeAmount); ``` --- ```solidity=57 token.transferFrom(msg.sender, address(this), amount); ``` --- ```solidity=152 token.transfer(msg.sender, freeAmount); ``` --- ```solidity=45 token.transferFrom(msg.sender, address(this), amount); ``` --- ```solidity=108 token.transfer(msg.sender, amount); ``` --- ```solidity=61 token.transferFrom(msg.sender, contractorAddr, amount); ``` --- ```solidity=122 token.transfer(msg.sender, freeAmount); ```

LowUnlocked Compiler VersionFixed

The contract has 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 an 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 Return ValueFixed

The return value of an external call is not stored in a local or state variable. ```solidity=59 token.icoTransferFrom(msg.sender, contractorAddr, amount); ``` --- ```solidity=60 token.icoTransferFrom(msg.sender, contractorAddr, amount); ``` --- ```solidity=79 token.increaseAllowance(address(this), burnAmount); ``` --- ```solidity=153 token.increaseAllowance(msg.sender, freeAmount); ```

LowUnused StructFixed

```solidity=18 struct Transaction { ``` - `Transaction` is declared but never used.

LowUpdate `lastTime` MistakenlyFixed

The `lastTime` is update mistakenly. Take the contract `MudAngelRoundReleaseBank` as an example: The token is released every day, so the released amount should be `(now - lastTime) / secPerDay * dailyReleaseAmount`. Suppose the locked token can be released from yesterday. Bob called `releaseToken()` at 8 am yesterday. The `lastTime` was updated to 8 am yesterday. The second time Bob will call `releaseToken()` at 8pm today. Now the `lastTime` was updated to 8 pm today. If the time that Bob calls `releaseToken()` is not the same every day, it may cause that the locked time will be longer than it expected. File `icoReleaseAngelRound.sol` ```solidity function releaseToken() public returns (uint256, uint256) { require(msg.sender != admin, "msg.send == admin"); require(bank[msg.sender].balance > 0, "balance <= 0"); require(now > bank[msg.sender].lastTime + secPerDay, "now < lastTime"); //calculate free amount uint256 freeAmount = (now - bank[msg.sender].lastTime) / secPerDay * bank[msg.sender].dailyReleaseAmount; if (freeAmount > bank[msg.sender].balance) { freeAmount = bank[msg.sender].balance; } bank[msg.sender].lastTime = now; bank[msg.sender].balance = bank[msg.sender].balance.sub(freeAmount); token.transfer(msg.sender, freeAmount); return (freeAmount, bank[msg.sender].balance); } ```

LowVariables That Could Be Declared as ImmutableFixed

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.