code-423n4 / 2022-10-thegraph-findings

0 stars 0 forks source link

QA Report #203

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago
  1. 0 address checks should be done

One should always check for 0 addresses in case the contract is initialized by zero address by mistake. Like in the following line

https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/governance/Governed.sol#L32

When the governor is first set, it could happen that _initGovernor is address(0) which would lead to inability to change governor.

Note: While this is an internal function, the caller of the function also does not check for 0 address.

Mitigation: Add the following line: require(_initGovernor) != address(0)

  1. Avoid assembly whenever required assembly{ id := chainid() } => uint256 id = block.chainid

Instance: https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/l2/token/GraphTokenUpgradeable.sol#L199

  1. It is a good programming practice to use constants instead of magic numbers

A constant variable could be defined for each hex value.

pcarranzav commented 1 year ago

block.chainid actually does not exist on solidity 0.7.6 (I've seen several QA reports suggesting we use it)