Anubis - Inadequate Access Control - StakingManager Role

[sherlock-admin]

Anubis

high

Inadequate Access Control - StakingManager Role

Summary

The contract lacks robust access control for critical functions that can alter the staking mechanism. The use of a single stakingManager account for sensitive operations without a multi-signature requirement or additional checks can lead to potential risks if the stakingManager account is compromised.

Vulnerability Detail

The contract utilizes a stakingManager address for important operations such as rewardValidators, disableValidator, enableValidator, addValidator, and setValidatorAddress. The current implementation does not enforce multi-signature requirements or other safeguards, making these operations susceptible to unilateral decisions or potential security breaches if the stakingManager key is compromised.

Impact

If the stakingManager is compromised or behaves maliciously, it could lead to unauthorized or improper validation management. This might include enabling/disabling validators arbitrarily, misappropriating funds, or manipulating the staking and reward mechanisms, leading to loss of funds or trust in the staking process.

Code Snippet

https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L262 https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L324 https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L333 https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L242 https://github.com/sherlock-audit/2023-11-covalent/blob/main/cqt-staking/contracts/OperationalStaking.sol#L689

Tool used

Manual Review

Recommendation

Implement a more robust access control mechanism. This could include multi-signature requirements for critical operations or a role-based access control system (RBAC) to distribute control over sensitive functions across multiple trusted parties. Here's a code snippet illustrating how you might introduce a multi-signature mechanism for critical functions:

// Define a structure for managing multi-signature requests
struct MultiSigRequest {
    uint256 id;
    bytes data;
    address[] confirmations;
    bool executed;
}

mapping(uint256 => MultiSigRequest) public multiSigRequests;
uint256 public requestCount;
uint256 public requiredConfirmations;

// Function to propose a new multi-signature request
function proposeRequest(bytes memory data) external onlyOwner {
    uint256 requestId = requestCount++;
    MultiSigRequest storage request = multiSigRequests[requestId];
    request.id = requestId;
    request.data = data;
    request.confirmations.push(msg.sender);
}

// Function to confirm a multi-signature request
function confirmRequest(uint256 requestId) external onlyOwner {
    MultiSigRequest storage request = multiSigRequests[requestId];
    require(!request.executed, "Request already executed");

    for (uint256 i = 0; i < request.confirmations.length; i++) {
        require(request.confirmations[i] != msg.sender, "Already confirmed");
    }

    request.confirmations.push(msg.sender);
    if (request.confirmations.length >= requiredConfirmations) {
        executeRequest(requestId);
    }
}

// Function to execute a confirmed multi-signature request
function executeRequest(uint256 requestId) internal {
    MultiSigRequest storage request = multiSigRequests[requestId];
    require(!request.executed, "Request already executed");
    require(request.confirmations.length >= requiredConfirmations, "Insufficient confirmations");

    (bool success, ) = address(this).call(request.data);
    require(success, "Transaction execution failed");

    request.executed = true;
}

// Initialize required confirmations in the constructor or a separate initialization function
constructor() {
    requiredConfirmations = 2; // for example, require signatures from 2 owners
}

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

[noslav]

Multisig requirements are implemented for StakingManager role which is specifically initially designed to be operated by a single unexposed privileged key, we note this recommendation and the centralization risk however it cannot be multi sig controlled due to the nature of its interactions being fluid and based on events being emitted by the contracts

[nevillehuang]

Invalid, there are necessary modifiers in place for permissions