Oracles Manual Review Findings

ORA-01M: Inexistent Validation of Signature Payload Submitter

Description:

The various signature-based functions of the Oracles implementation do not validate the msg.sender and thus allow anyone to submit a set of valid signatures that will result in the corresponding action being executed. While this allows for versatility, it enables complex attacks to unfold by an attacker inspecting the mempool, identifying the action being performed and executing it themselves with transactions before and after it that would normally be impossible, such as flash loans. An example of this would be the significant dilution of the new reward-per-token increase by a user inspecting the mempool for a valid submitRewards invocation, making a flash loan deposit to a Pool and in turn to StakedEthToken thus diluting the rewards. While the impact is offset by the maximum pending validators threshold, it is still an example of what permissionless submission of vote executions can lead to.

Example:

Copy
142/**
143 * @dev See {IOracles-submitRewards}.
144 */
145function submitRewards(
146    uint256 totalRewards,
147    uint256 activatedValidators,
148    bytes[] memory signatures
149)
150    external override whenNotPaused
151{
152    require(
153        signatures.length.mul(3) > getRoleMemberCount(ORACLE_ROLE).mul(2),
154        "Oracles: invalid number of signatures"
155    );
156
157    // calculate candidate ID hash
158    uint256 nonce = rewardsNonce.current();
159    bytes32 candidateId = ECDSAUpgradeable.toEthSignedMessageHash(
160        keccak256(abi.encode(nonce, activatedValidators, totalRewards))
161    );
162
163    // check signatures and calculate number of submitted oracle votes
164    address[] memory signedOracles = new address[](signatures.length);
165    for (uint256 i = 0; i < signatures.length; i++) {
166        bytes memory signature = signatures[i];
167        address signer = ECDSAUpgradeable.recover(candidateId, signature);
168        require(hasRole(ORACLE_ROLE, signer), "Oracles: invalid signer");
169
170        for (uint256 j = 0; j < i; j++) {
171            require(signedOracles[j] != signer, "Oracles: repeated signature");
172        }
173        signedOracles[i] = signer;
174        emit RewardsVoteSubmitted(msg.sender, signer, nonce, totalRewards, activatedValidators);
175    }
176
177    // increment nonce for future signatures
178    rewardsNonce.increment();
179
180    // update total rewards
181    rewardEthToken.updateTotalRewards(totalRewards);
182
183    // update activated validators
184    if (activatedValidators != pool.activatedValidators()) {
185        pool.setActivatedValidators(activatedValidators);
186    }
187}

Recommendation:

We advise the functions handling signatures payloads to either be invoked only by existing ORACLE_ROLE members or by ensuring that the invocator is equal to the tx.origin. The latter would be a temporary solution as EIP-3074 will deprecate this security feature, however, it will be valid for at least the foreseeable future (over 6 month lifetime) given that it would be a consortium upgrade. Should it be applied, we advise the Stakewise team to simply monitor upcoming Ethereum upgrades and adjust the code as necessary given that the upgrade-able nature of the contract permits them to.

Alleviation:

Caller validation was introduced to the sensitive subset of functions exposed by the contracts thus alleviating this exhibit.

ORA-02M: Single Point of Failure

Description:

The contract suffers from a SPoF whereby an oracle's membership is completely dictated by either the role administrator or the administrator of the contract which is able to grant such a role. This can affect consortiums and to that extent all votes processed via the system.

Example:

Copy
118/**
119 * @dev See {IOracles-addOracle}.
120 */
121function addOracle(address account) external override {
122    grantRole(ORACLE_ROLE, account);
123    emit OracleAdded(account);
124}
125
126/**
127 * @dev See {IOracles-removeOracle}.
128 */
129function removeOracle(address account) external override {
130    revokeRole(ORACLE_ROLE, account);
131    emit OracleRemoved(account);
132}

Recommendation:

We advise this trait to be carefully examined and if deemed undesirable, we advise the inclusion and removal of new oracles to be performed via an on-chain vote instead.

Alleviation:

The Stakewiste team stated that the administrator of the system is the Stakewise DAO which can only perform actions after votes have been processed and properly timelocked. As a result, we consider this exhibit dealt with.