3525Lottery - Audit (Addendum)

CertiK6 Feb 2023Files named in the report

CertiK reviewed 3525 Lottery (3525Lottery - Audit (Addendum)) on 6 Feb 2023: 16 findings, 0 critical; 1 acknowledged, 15 open.

No open critical findings

Open the report Open 3525 Lottery’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings16

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

Severity	Open	Acknowledged
Critical	0	0
High	0	1
Medium	0	0
Low	15	0

LowDivide Before MultiplyOpen

Performing integer division before multiplication truncates the low bits, losing the precision of calculation. ```solidity=63 uint profit = _value.div(ONE_UINTS).mul(RATIO[i]).mul(promote).div(10); ``` --- ```solidity=494 return ONCE_PRICE.mul(10 ** 10).div(_p_stage).mul(_counts); ``` --- ```solidity=702 uint shares = ONCE_PRICE.mul(10 ** 10).div(_p_stage).mul(_counts); ``` --- ```solidity=713 uint per = _value / 100; ``` ```solidity=715 gainPool += per * 10; ```

LowIncorrect Error MessageOpen

The error message in `require(!_pause, "draw suspended");` does not describe the error correctly.

LowLocked EtherOpen

The contract has one or more payable functions, but does not have a function to withdraw the fund.

LowMissing Emit EventsOpen

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

LowMissing Zero Address ValidationOpen

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=26 OWNER = _owner; ``` - `_owner` is not zero-checked before being used. --- ```solidity=334 manager = _address; ``` - `_address` is not zero-checked before being used. --- ```solidity=25 manager = _manager; ``` - `_manager` is not zero-checked before being used.

LowRedundant Code ComponentsOpen

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

LowState Variable Should Be Declared ConstantOpen

State variables that never change should be declared as `constant` to save gas. ```solidity=19 uint decimals = 6; ``` - `decimals` should be declared `constant`. --- ```solidity=48 uint8 decimals = 6; // usdt decimals ``` - `decimals` should be declared `constant`. --- ```solidity=55 uint public SHARES_SLOT_ID = 100; // SFT slot ID ``` - `SHARES_SLOT_ID` should be declared `constant`. --- ```solidity=88 uint64 private subscriptionId = 539; ``` - `subscriptionId` should be declared `constant`. --- ```solidity=89 uint32 private callbackGasLimit = 400000; ``` - `callbackGasLimit` should be declared `constant`. --- ```solidity=90 uint16 private requestConfirmations = 3; ``` - `requestConfirmations` should be declared `constant`. --- ```solidity=91 uint32 private numWords = 1; ``` - `numWords` should be declared `constant`. --- ```solidity=92 bytes32 private keyHash = 0xcc294a196eeeb44da2888d17c0625cc88d70d9760a69d58d853ba6581a9ab0cd; ``` - `keyHash` should be declared `constant`. --- ```solidity=93 address private coordinator = 0xAE975071Be8F8eE67addBC1A82488F1C24858067; ``` - `coordinator` should be declared `constant`.

LowThird Party DependencyOpen

The contract is serving as the underlying entity to interact with one or more third party protocols. The scope of the audit treats third party entities as black boxes and assume their functional correctness. However, in the real world, third parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of third parties can possibly create severe impacts, such as increasing fees of third parties, migrating to new LP pools, etc. ```solidity=25 IERC20 private immutable USDT; ``` - The contract `Lottery` interacts with third party contract with `IERC20` interface via `USDT`. --- ```solidity=87 VRFCoordinatorV2Interface COORDINATOR; ``` - The contract `Lottery` interacts with third party contract with `VRFCoordinatorV2Interface` interface via `COORDINATOR`.

LowThird-party IPFS AccessOpen

Even though the `_contractImage()` function is returning an image that is indeed stored on IPFS (it was verified by accessing it directly using IPFS). It is using the Infura third-party service to . This finding was rated as low risk because the real image would always be accessible using an alternative HTTPS URL https://ipfs.io/ipfs/QmV643pavJTWyi6Q7tP25bCLQ8dBp5YmYrEqSf7ZWYiAca or using IPFS directly.

LowUnlocked Compiler VersionOpen

The contracts cited have 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. Furthermore, different Solidity versions are used in the codebase.

Show 5 more

LowUnnecessary Use of SafeMathOpen

The `SafeMath` library is used unnecessarily. With Solidity compiler versions 0.8.0 or newer, arithmetic operations will automatically revert in case of integer overflow or underflow. ```solidity=13 using SafeMath for uint; ``` - `SafeMath` library is used for `uint256` type in `Agency` contract. ```solidity=63 uint profit = _value.div(ONE_UINTS).mul(RATIO[i]).mul(promote).div(10); ``` - `SafeMath.div` is called in `calculateIncome` function of `Agency` contract. *Note: Only a sample of 2 `SafeMath` library usage in this contract (out of 5) are shown above.* --- ```solidity=16 using SafeMath for uint; ``` - `SafeMath` library is used for `uint256` type in `Arithmetic` contract. ```solidity=37 uint word = random.mod(MOD); ``` - `SafeMath.mod` is called in `getResult` function of `Arithmetic` contract. *Note: Only a sample of 2 `SafeMath` library usage in this contract (out of 9) are shown above.* --- ```solidity=22 using SafeMath for uint; ``` - `SafeMath` library is used for `uint256` type in `Lottery` contract. ```solidity=196 uint amount = _value.mul(sharesPool).div(totalSupplyShares); ``` - `SafeMath.mul` is called in `burnShares` function of `Lottery` contract. *Note: Only a sample of 2 `SafeMath` library usage in this contract (out of 19) are shown above.*

LowUnused EventOpen

```solidity=12 event PriceChange(uint value); ``` - `PriceChange` is declared in `ILottery` but never emitted.

LowUnused State VariableOpen

One or more state variables are never used in the codebase. Variable `ONE_UINTS` in `Lottery` is never used in `Lottery`. ```solidity=51 uint private ONE_UINTS = 1 * 10 ** decimals; // 1 usdt = 1000000 ``` ```solidity=20 contract Lottery is VRFConsumerBaseV2, ILottery, Roles { ``` --- Variable `__gap` in `ERC3525` is never used in `LotterySFT`. ```solidity=674 uint256[42] private __gap; ``` ```solidity=9 contract LotterySFT is ERC3525SlotEnumerable, Ownable { ```

LowVariables That Could Be Declared as ImmutableOpen

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.

LowVisibility Specifiers MissingOpen

The linked variable declarations do not have a visibility specifier explicitly set. Inconsistencies in the default visibility the Solidity compilers impose can cause issues in the functionality of the codebase.