scheduleRely can be executed immediately defeating the Time ward allocation purpose

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/Root.sol#L65

Vulnerability details

The use of MAX_DELAY to set the maximum amount of time that a scheduledRely can last is correct but there is actually no way to use a minimal time to limit the time that a contract or address has to wait before they can actually become a ward. I assume the Time ward allocation was made to verify the integrity of the contract trying to become a ward before granting them permission

let ‘s check out a scenario

An attacker, Alice, gains access to one of the initial ward accounts in the protocol

Alice, with her newfound access, uses the protocol's relyContract function to add a new address, "Malicious1," as a ward.

Malicious1, now having ward privileges, proceeds to create additional rogue wards by using the same relyContract function. Malicious1 creates "Malicious2" as a ward.

Since there's no wait time before a ward can be accepted as a ward, all these dangerous appointment can come into effect and become a ward before other wards realise

Proof of Concept

  uint256 private MAX_DELAY = 4 weeks;
/// --- Timelocked ward management ---
    function scheduleRely(address target) external auth {
        schedule[target] = block.timestamp + delay;
        emit RelyScheduled(target, schedule[target]);
    }

    function cancelRely(address target) external auth {
        schedule[target] = 0;
        emit RelyCancelled(target);
    }

    function executeScheduledRely(address target) public {
        require(schedule[target] != 0, "Root/target-not-scheduled");
        require(schedule[target] < block.timestamp, "Root/target-not-ready");

        wards[target] = 1;
        emit Rely(target);

        schedule[target] = 0;
    }

You can see that when you actually try to ScheduleRely there is no minimum TIME_DELAY to actually have a required duration of time to verify the integrity of the ward that is being added, if its malicious or not.

Recommended Mitigation Steps

The same way you set the MAX_DELAY to prevent a long amount of time before executing the rely, you should also set a MIN_DELAY to verify the integrity of the address or contract before execution

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #298

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)

[c4-judge]

gzeon-c4 marked the issue as grade-c