search

code-423n4 / 2022-06-nested-findings

0 stars 1 forks source link

Missing zero address check could lead to an executeEmergency() callable by anyone #17

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nested/blob/main/contracts/governance/TimelockControllerEmergency.sol#L295

Vulnerability details

Impact

The executeEmergency() is only callable by the EMERGENCY_ROLE role. However if at constract construction, emergency address was is zero, then EMERGENCY_ROLE role would be enabled for everyone leading to anyone executing a transaction without scheduling

Proof of Concept

  1. Assume the contract owner deployed the contract with the inputing emergency argument in TimelockControllerEmergency.constructor() as address(0) hoping to change it anytime.
  2. With the logic of @openzeppelin/contracts/access/AccessControl.sol , this would enable the EMERGENCY_ROLE role open to everyone.
  3. Bob who has no assigned role, calls executeEmergency() without restriction and can execute a transaction without any delay.

Tools Used

Manual review

Recommended Mitigation Steps

Add a require check for zero address in the constructor function.

Yashiru commented 2 years ago

Missing zero address check could lead to an executeEmergency() callable by anyone (Duplicated)

Duplicated of #61 at 2. Missing address(0) checks