Closed code423n4 closed 1 year ago
https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/l2/token/GraphTokenUpgradeable.sol#L105-L108
The governor can add as many minter addresses as he/she wants.
105: function addMinter(address _account) external onlyGovernor { 106: require(_account != address(0), "INVALID_MINTER"); 107: _addMinter(_account); 108: }
I believe this presents an important risk as if one of these minter addresses is compromised, unlimited number of tokens can be minted.
Manual review
Consider giving the minter role to only the governor. Since, the governor is set within the initialize() function, the risks would be mostly eliminated.
initialize()
Lines of code
https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/l2/token/GraphTokenUpgradeable.sol#L105-L108
Vulnerability details
Impact
The governor can add as many minter addresses as he/she wants.
I believe this presents an important risk as if one of these minter addresses is compromised, unlimited number of tokens can be minted.
Proof of Concept
https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/l2/token/GraphTokenUpgradeable.sol#L105-L108
Tools Used
Manual review
Recommended Mitigation Steps
Consider giving the minter role to only the governor. Since, the governor is set within the
initialize()
function, the risks would be mostly eliminated.