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

0 stars 0 forks source link

QA Report #273

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

QA report

Low Risk

L-N Issue
[L‑01] Wrong event parameter emit
[L‑02] With only admin role can't acceptUpgrade or acceptUpgradeAndCall
[L‑03] Mark as abstract

Non-critical

N-N Issue
[N‑01] Missing error message in require statement
[N‑02] Unused imports
[N‑03] Non-library/interface files should use fixed compiler versions, not floating ones
[N‑04] Wrong function doc

Low Risk

[L-01] Wrong event parameter emit

The _admin() will be the new admin and not the old, save the old admin in a var and emit this one, like in _setImplementation or _setPendingImplementation functions

File: /contracts/upgrades/GraphProxyStorage.sol

86        emit AdminUpdated(_admin(), _newAdmin);

[L-02] With only admin role can't acceptUpgrade or acceptUpgradeAndCall

The functions acceptUpgrade and acceptUpgradeAndCall have the ifAdminOrPendingImpl modifier, but it the sender it's the _admin the transcaction recerts the msg.sender == _pendingImplementation inside the _acceptUpgrade function

File: /contracts/upgrades/GraphProxy.sol

122    function acceptUpgrade() external ifAdminOrPendingImpl {

129    function acceptUpgradeAndCall(bytes calldata data) external ifAdminOrPendingImpl {

143            _pendingImplementation != address(0) && msg.sender == _pendingImplementation,

[L-03] Mark as abstract

If the Governed.sol contract is deployed, will fail to initialize

File: /contracts/governance/Governed.sol

9 contract Governed {

Same with the GraphTokenUpgradeable.sol

File: /contracts/l2/token/GraphTokenUpgradeable.sol

28 contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgradeable {
File: /contracts/upgrades/GraphUpgradeable.sol

11 contract GraphUpgradeable {
File: /contracts/upgrades/GraphProxyStorage.sol

11 contract GraphProxyStorage {
File: /contracts/governance/Managed.sol

23 contract Managed {

Non-critical

[N-01] Missing error message in require statements

File: /contracts/upgrades/GraphProxy.sol

133        require(success);
File: /contracts/upgrades/GraphProxyAdmin.sol

34        require(success);

47        require(success);

59        require(success);

[N-02] Unused imports

File: /contracts/curation/ICuration.sol

5 import "./IGraphCurationToken.sol";

[N‑03] Non-library/interface files should use fixed compiler versions, not floating ones

File: /contracts/gateway/BridgeEscrow.sol

3 pragma solidity ^0.7.6;
File: /contracts/upgrades/GraphUpgradeable.sol

3 pragma solidity ^0.7.6;
File: /contracts/governance/Governed.sol

3 pragma solidity ^0.7.6;
File: /contracts/governance/Pausable.sol

3 pragma solidity ^0.7.6;
File: /contracts/l2/token/L2GraphToken.sol

3 pragma solidity ^0.7.6;
File: /contracts/upgrades/GraphProxyAdmin.sol

3 pragma solidity ^0.7.6;
File: /contracts/upgrades/GraphProxy.sol

3 pragma solidity ^0.7.6;
File: /contracts/governance/Managed.sol

3 pragma solidity ^0.7.6;
File: /contracts/l2/token/GraphTokenUpgradeable.sol

3 pragma solidity ^0.7.6;
File: /contracts/l2/gateway/L2GraphTokenGateway.sol

3 pragma solidity ^0.7.6;
File: /contracts/gateway/L1GraphTokenGateway.sol

3 pragma solidity ^0.7.6;
File: /contracts/gateway/GraphTokenGateway.sol

3 pragma solidity ^0.7.6;
File: /contracts/curation/IGraphCurationToken.sol

3 pragma solidity ^0.7.6;
File: /contracts/gateway/ICallhookReceiver.sol

9 pragma solidity ^0.7.6;
File: /contracts/upgrades/IGraphProxy.sol

3 pragma solidity ^0.7.6;
File: /contracts/epochs/IEpochManager.sol

3 pragma solidity ^0.7.6;
File: /contracts/governance/IController.sol

3 pragma solidity >=0.6.12 <0.8.0;
File: /contracts/token/IGraphToken.sol

3 pragma solidity ^0.7.6;
File: /contracts/rewards/IRewardsManager.sol

3 pragma solidity ^0.7.6;
File: /contracts/staking/IStakingData.sol

3 pragma solidity >=0.6.12 <0.8.0;
File: /contracts/curation/ICuration.sol

3 pragma solidity ^0.7.6;
File: /contracts/staking/IStaking.sol

3 pragma solidity >=0.6.12 <0.8.0;

[N‑04] Wrong function doc

Should be: * NOTE: Only the admin and implementation can call this function. like in admin function

File: /contracts/upgrades/GraphProxy.sol

76     * NOTE: Only the admin can call this function.

89     * NOTE: Only the admin can call this function.
pcarranzav commented 1 year ago

L-01 and L-02 are good finds!