windranger-io / bit-token-contract

The BitDAO BIT token contract as deployed on Mainnet
https://etherscan.io/address/0x1A4b46696b2bB4794Eb3D4c26f1c55F9170fa4C5
Apache License 2.0
2 stars 1 forks source link

Slither static code analysis #20

Open CjHare opened 3 years ago

CjHare commented 3 years ago

Result of running the Slither analysis against Solidity 0.7.6.

42 result(s) found

ethsec@8f68a22109dd:/home/bitdao$ slither .
'npx hardhat compile --force' running
Compiling 1 file with 0.7.6
Generating typings for: 8 artifacts in dir: typechain for target: ethers-v5
Successfully generated 9 typings!
Compilation finished successfully

contracts/BitDAO.sol:512:5: Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
    constructor (string memory name_, string memory symbol_) public {
    ^ (Relevant source part starts here and spans across multiple lines).

BitDAO._writeCheckpoint(address,uint256,uint256,uint256) (contracts/BitDAO.sol#1042-1058) uses a dangerous strict equality:
    - nCheckpoints > 0 && checkpoints[delegatee][nCheckpoints - 1].fromBlock == blockNumber (contracts/BitDAO.sol#1050)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#dangerous-strict-equalities

BitDAO.constructor(address)._admin (contracts/BitDAO.sol#831) lacks a zero-check on :
        - admin = _admin (contracts/BitDAO.sol#832)
BitDAO.setPendingAdmin(address).newPendingAdmin (contracts/BitDAO.sol#836) lacks a zero-check on :
        - pendingAdmin = newPendingAdmin (contracts/BitDAO.sol#841)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#missing-zero-address-validation

BitDAO.delegateBySig(address,uint256,uint256,uint8,bytes32,bytes32) (contracts/BitDAO.sol#945-962) uses timestamp for comparisons
    Dangerous comparisons:
    - require(bool,string)(block.timestamp <= expiry,BitDAO::delegateBySig: signature expired) (contracts/BitDAO.sol#960)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp

BitDAO.getChainId() (contracts/BitDAO.sol#1065-1071) uses assembly
    - INLINE ASM (contracts/BitDAO.sol#1067-1069)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#assembly-usage

BitDAO._beforeTokenTransfer(address,address,uint256) (contracts/BitDAO.sol#883-902) is never used and should be removed
BitDAO._lastSnapshotId(uint256[]) (contracts/BitDAO.sol#933-939) is never used and should be removed
BitDAO._transfer(address,address,uint256) (contracts/BitDAO.sol#1011-1018) is never used and should be removed
BitDAO._updateAccountSnapshot(address) (contracts/BitDAO.sol#917-919) is never used and should be removed
BitDAO._updateSnapshot(BitDAO.Snapshots,uint256) (contracts/BitDAO.sol#925-931) is never used and should be removed
BitDAO._updateTotalSupplySnapshot() (contracts/BitDAO.sol#921-923) is never used and should be removed
Context._msgData() (contracts/BitDAO.sol#370-373) is never used and should be removed
Counters.decrement(Counters.Counter) (contracts/BitDAO.sol#344-346) is never used and should be removed
ERC20._burn(address,uint256) (contracts/BitDAO.sol#706-714) is never used and should be removed
ERC20._setupDecimals(uint8) (contracts/BitDAO.sol#744-746) is never used and should be removed
Math.max(uint256,uint256) (contracts/BitDAO.sol#235-237) is never used and should be removed
Math.min(uint256,uint256) (contracts/BitDAO.sol#242-244) is never used and should be removed
SafeMath.div(uint256,uint256) (contracts/BitDAO.sol#141-144) is never used and should be removed
SafeMath.div(uint256,uint256,string) (contracts/BitDAO.sol#196-199) is never used and should be removed
SafeMath.mod(uint256,uint256) (contracts/BitDAO.sol#158-161) is never used and should be removed
SafeMath.mod(uint256,uint256,string) (contracts/BitDAO.sol#216-219) is never used and should be removed
SafeMath.mul(uint256,uint256) (contracts/BitDAO.sol#122-127) is never used and should be removed
SafeMath.tryAdd(uint256,uint256) (contracts/BitDAO.sol#30-34) is never used and should be removed
SafeMath.tryDiv(uint256,uint256) (contracts/BitDAO.sol#66-69) is never used and should be removed
SafeMath.tryMod(uint256,uint256) (contracts/BitDAO.sol#76-79) is never used and should be removed
SafeMath.tryMul(uint256,uint256) (contracts/BitDAO.sol#51-59) is never used and should be removed
SafeMath.trySub(uint256,uint256) (contracts/BitDAO.sol#41-44) is never used and should be removed
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code

Pragma version>=0.6.5<0.8.0 (contracts/BitDAO.sol#768) is too complex
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity

Variable BitDAO.MAX_SUPPLY (contracts/BitDAO.sol#780) is not in mixedCase
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#conformance-to-solidity-naming-conventions

Redundant expression "this (contracts/BitDAO.sol#371)" inContext (contracts/BitDAO.sol#365-374)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#redundant-statements

BitDAO.MAX_SUPPLY (contracts/BitDAO.sol#780) should be constant
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant

symbol() should be declared external:
    - ERC20.symbol() (contracts/BitDAO.sol#529-531)
decimals() should be declared external:
    - ERC20.decimals() (contracts/BitDAO.sol#546-548)
transfer(address,uint256) should be declared external:
    - ERC20.transfer(address,uint256) (contracts/BitDAO.sol#572-575)
allowance(address,address) should be declared external:
    - ERC20.allowance(address,address) (contracts/BitDAO.sol#580-582)
approve(address,uint256) should be declared external:
    - ERC20.approve(address,uint256) (contracts/BitDAO.sol#591-594)
transferFrom(address,address,uint256) should be declared external:
    - ERC20.transferFrom(address,address,uint256) (contracts/BitDAO.sol#609-613)
increaseAllowance(address,uint256) should be declared external:
    - ERC20.increaseAllowance(address,uint256) (contracts/BitDAO.sol#627-630)
decreaseAllowance(address,uint256) should be declared external:
    - ERC20.decreaseAllowance(address,uint256) (contracts/BitDAO.sol#646-649)
balanceOfAt(address,uint256) should be declared external:
    - BitDAO.balanceOfAt(address,uint256) (contracts/BitDAO.sol#871-875)
totalSupplyAt(uint256) should be declared external:
    - BitDAO.totalSupplyAt(uint256) (contracts/BitDAO.sol#877-881)
getPriorVotes(address,uint256) should be declared external:
    - BitDAO.getPriorVotes(address,uint256) (contracts/BitDAO.sol#969-999)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external
. analyzed (8 contracts with 75 detectors), 42 result(s) found
CjHare commented 3 years ago

Better organisation of the findings in their categories, mainly low & optimisation issues.

ethsec@8f68a22109dd:/home/bitdao$ slither contracts/BitDAO.sol --print human-summary
Compilation warnings/errors on contracts/BitDAO.sol:
Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient.
   --> contracts/BitDAO.sol:512:5:
    |
512 |     constructor (string memory name_, string memory symbol_) public {
    |     ^ (Relevant source part starts here and spans across multiple lines).

Compiled with solc
Number of lines: 1072 (+ 0 in dependencies, + 0 in tests)
Number of assembly lines: 0
Number of contracts: 8 (+ 0 in dependencies, + 0 tests)

Number of optimization issues: 12
Number of informational issues: 26
Number of low issues: 3
Number of medium issues: 1
Number of high issues: 0

ERCs: ERC20

+----------+-------------+-------+--------------------+--------------+-----------+
|   Name   | # functions |  ERCS |     ERC20 info     | Complex code |  Features |
+----------+-------------+-------+--------------------+--------------+-----------+
| SafeMath |      13     |       |                    |      No      |           |
|   Math   |      3      |       |                    |      No      |           |
|  Arrays  |      1      |       |                    |     Yes      |           |
| Counters |      3      |       |                    |      No      |           |
|  BitDAO  |      50     | ERC20 |     No Minting     |     Yes      | Ecrecover |
|          |             |       | Approve Race Cond. |              |  Assembly |
|          |             |       |                    |              |           |
+----------+-------------+-------+--------------------+--------------+-----------+
contracts/BitDAO.sol analyzed (8 contracts)