sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

mau - TimeController contract enables inappropriate adjustment of minimum delay #151

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

mau

medium

TimeController contract enables inappropriate adjustment of minimum delay

Summary

The TimeController contract has a vulnerability that allows unauthorized modification of the minimum delay, potentially leading to unintended changes in critical system settings.

Vulnerability Detail

The TimeController contract lacks a sanity check when roles with permission attempt to set the minimum delay. While the documentation specifies a delay of 24 or 48 hours, there is no validation mechanism in place to ensure that the roles do not set the delay to lower values.

Impact

The absence of a sanity check in the TimeController contract exposes the system to the risk of incorrect delay settings. Roles with permission can set the minimum delay to values lower than the intended threshold, leading to unexpected timing behaviors. This vulnerability may disrupt critical state changes, compromise system security, and undermine the stability and reliability of the overall system.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/d5328421bea80e3b0fd4595e4eb6b732a40e421e/Unitas-Protocol/src/TimelockController.sol#L23

Tool used

Manual Review

Recommendation

To mitigate the vulnerability, it is strongly recommended to implement a sanity check in the TimeController contract. This sanity check should validate that the minimum delay set by the specified roles falls within the allowed range (24 or 48 hours). If the minimum delay is set to a value lower than the threshold, the contract should reject the change and revert to the previous valid setting.

    modifier validDelay(uint256 newDelay) {
        require(newDelay >= _MINIMUM_DELAY && newDelay <= _MAXIMUM_DELAY, "TimelockController: invalid delay");
        _;
    }

The validDelay modifier ensures that the new delay value provided in the updateDelay function is within the allowed range. If the new delay is not within the range, the contract will revert and the change will not be applied.