SteerGovernance Manual Review Findings

SGE-01M: Logic Upgrade w/o Governance Approval

Type	Severity	Location
Centralization Concern		SteerGovernance.sol:L52-L56

Description:

The SteerGovernance contract represents an upgradeable contract and the way its ownership is established the contract can be upgraded without a voting process taking place.

Example:

Copy
32function initialize(
33    ERC20VotesUpgradeable _token,
34    TimelockControllerUpgradeable _timelock
35) public initializer {
36    __Governor_init("SteerGovernance");
37    __GovernorSettings_init(
38        1, /* 1 block of voting delay*/
39        45818, /* Number of blocks inside voting period */
40        100e18 /* Voters need 100 tokens to vote */
41    );
42    __GovernorCompatibilityBravo_init();
43    __GovernorVotes_init(_token);
44    __GovernorVotesQuorumFraction_init(
45        40 /* 40% */
46    );
47    __GovernorTimelockControl_init(_timelock);
48    __Ownable_init();
49    __UUPSUpgradeable_init();
50}
51
52function _authorizeUpgrade(address newImplementation)
53    internal
54    override
55    onlyOwner
56{}

Recommendation:

We advise either the ownership of the contract to be transferred to its timelock or the contract itself to no longer be upgradeable, the latter of which we advise as governance processes should remain as immutable as possible in the lifetime of a project.

Alleviation (0ed41ccc18a72b7e559b8d79ab7ba6172362ee3b):

The Steer Protocol has stated that they wish to retain the current ownership and upgradeability system in place as they may require new features to be introduced to the governance module prior to its eventual complete decentralization and ownership transfer. As a result, we consider this exhibit acknowledged.

SGE-02M: Inexistent Access Control of Internal Governance

Type	Severity	Location
Logical Fault		SteerGovernance.sol:L171-L179

Description:

The setInternalGovernanceTimeLockOnce function does not impose any access control on its caller thus leading to an on-chain race-condition materializing.

Impact:

Given that the SteerGovernance initialization is a two-step process (first the initialize function is invoked and then the setInternalGovernanceTimeLockOnce function), the latter of the two is prone to a race condition whereby bots can detect the contract being deployed and attempt to initialize it to a malicious contract.

Example:

Copy
168/// @dev Use this function to set address of Internal Governance Timelock.
169/// @dev This function can only be called once while deployment.
170/// @param _internalGovernanceTimeLock is the address of Internal Governance Timelock.
171function setInternalGovernanceTimeLockOnce(
172    address _internalGovernanceTimeLock
173) external {
174    require(
175        InternalGovernanceTimeLock == address(0),
176        "Timelock is already set"
177    );
178    InternalGovernanceTimeLock = _internalGovernanceTimeLock;
179}

Recommendation:

We advise the function to apply onlyOwner capabilities to ensure that the InternalGovernanceTimeLock contract is not prone to an on-chain gas race due to multiple users attempting to initialize it first.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

The onlyOwner modifier has been introduced to the function thus disallowing an on-chain race condition to materialize and alleviating this exhibit in full.

SGE-03M: Abnormally Small Voting Delay

Type	Severity	Location
Logical Fault		SteerGovernance.sol:L38

Description:

The voting delay of the SteerGovernance contract is 1 block which is an abnormally low voting delay that hurts the operational integrity of the system.

Impact:

As an example, a user can take a significant loan albeit for a meagre two blocks after which they return it with minimal mark-up but completely compromise the voting process of the Steer Protocol.

Example:

Copy
37__GovernorSettings_init(
38    1, /* 1 block of voting delay*/
39    45818, /* Number of blocks inside voting period */
40    100e18 /* Voters need 100 tokens to vote */
41);

Recommendation:

We advise the voting delay to be adjusted to a higher value as it is currently possible for well-funded attackers to completely compromise the governance integrity of the contract.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

The voting delay has been increased to 14400 blocks which are equivalent to roughly 2 days of a delay, significantly increasing the operational security of the governance module and alleviating this exhibit.