Tometa72 DAO security record

No tokens can be generated within the updated files `Optimized_ERC20.sol` and `Optimized_DistributionERC20.sol`. This issue severely impacts the core functionality of the contracts, rendering them ineffective for their intended purpose. In a typical ERC20 token contract, mechanisms for token minting or initial token distribution are crucial to create the circulating supply of the token.

In the contract `ERC20` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and set wallets, set `_startTime`, transfer or renounce the ownership, and check wallets. 

In the contract `ERC20`, the `_lockBalanceNum()` function is intended to calculate the amount of tokens that are locked in a wallet. The contract's design expects that if a wallet's balance falls below this locked amount, any transfer attempts should be blocked. However, an issue arises in the `_transfer()` function, where a wallet can effectively reduce its locked token amount through unauthorized transfers. This vulnerability stems from a flaw in the `_lockBalanceNum()` function's calculation, which incorrectly allows the locked amount to be influenced by the wallet's actions, rather than strictly adhering to the current timestamp as intended.

In the contract `ERC20`, the `_lockBalanceNum()` function is intended to calculate the amount of tokens that are locked in a wallet. The line 751 in the function checks whether `walletRecorde.lastRelease` equals 0 as a means to determine if tokens have been previously released from the wallet. However, a discrepancy arises because `walletRecorde.lastRelease` might be updated before `_startTime`, erroneously indicating that the wallet has already initiated a token release. This premature update can lead to an overestimation of locked tokens, thus locking more tokens in the wallet than intended. ```solidity=750 //初次释放(证明还有初始释放) if (walletRecorde.lastRelease == 0) { ```

The `_lockBalanceNum()` function uses the lock duration to calculate the amount of locked tokens. It determines the duration by subtracting the `walletRecorde.lastRelease` timestamp, which represents the time when the wallet last transferred tokens, from the current timestamp (as observed in line 792 of the code). This method, however, may lead to an underestimation of the actual lock duration. The reason is that `walletRecorde.lastRelease` might not accurately reflect the beginning of the lock period. This miscalculation can potentially result in a larger number of tokens being locked than originally planned. ```solidity=792 duration = nowTime.sub(walletRecorde.lastRelease); ```

In the contract `ERC20`, the `_lockBalanceNum()` function is intended to calculate the amount of tokens that are locked in a wallet. The design intention is that the locked token amount should be 0 if the current timestamp (`nowTime`) is less than the specified `_startTime`. However, the function lacks a crucial check to verify whether `nowTime` is greater than `_startTime`. As a result, this oversight in the code logic leads to the premature release of tokens before the intended `_startTime`.

The `setOrganization()` function is responsible for allocating locked tokens to specified wallets. However, the function lacks a check to confirm whether a wallet has been previously set. This omission can lead to the inadvertent overwriting of existing records in `walletRecords[_walletArray[i]]`. Consequently, such overwriting might result in unintended behaviors. For example, most tokens in the wallet could be immediately released since the amount of locked tokens would be updated. ```solidity=428 for (uint256 i = 0; i < _walletArray.length; i++) { orgWallet[_organizationId].push(_walletArray[i]); walletRecords[_walletArray[i]] = WalletRecord( _amountArray[i], _amountArray[i], _organizationId, _firstUnlockPre, _releaseIntervalValue, _intervalTimeValue, _secondUnlockPre, 0, _releaseAllTime ); _transfer(_owner, _walletArray[i], _amountArray[i]); } ```

In the `ERC20` contract, the `_releaseAll()` function is tasked with calculating the timestamp (`_releaseAllTimes`) when all tokens become fully released. However, the calculation in the current code is incorrect. Taking the `orgOne` example, the function erroneously computes `_releaseAllTimes` as `_startTime + (11 * 30 days)`, while the expected outcome should be `_startTime + (10 * 30 days)`. ```solidity=725 uint256 times = (firstBalance.sub(firstBalance.mod(_secondUnlockPre))) .div(_secondUnlockPre); uint256 _releaseAllTimes = _startTime.add(_releaseInterval).add( times.mul(_intervalTime) ); ``` ```solidity uint256[5] orgOne = [200000000 * 10**18, 0, 1 * 30, 1 * 30, 100]; ``` --- Preliminary report only: The auditors would like to confirm that `_startTime + 1 * 30 days` is the first time of the second-round release, is that correct?

In the `_transfer()` function, the following comment suggests that the contract deployer is restricted to transferring tokens only once and exclusively to wallets within organizations. However, the actual implementation of the code enforces these restrictions on the `_owner` instead of the deployer. Furthermore, the existing `require` check within the code does not effectively enforce the one-time transfer limitation for the `_owner`. ```solidity=524 //部署合约的地址只能转给机构下的钱包,且只能转一次 if (from == _owner) { require( walletRecords[to].index != 0 && walletRecords[to].lastRelease == 0, "can not tx" ); } ```

The line 369 in the `setStartTime()` function requires `startTime >= block.timestamp`. However, the line 463 in the `_releaseToken()` requires `_startTime` to be larger than `nowTime` (block.timestamp) when `setStartTime()` is called. ```solidity=369 require(startTime >= block.timestamp, "time error"); ``` ```solidity=463 require(nowTime < _startTime, "not initial time"); ```