Liquid Staking

Summary

The PriorityPool contract has a vulnerability that allows the distributionOracle address to indirectly modify the merkleRoot state variable through the pauseForUpdate function, which emits a Paused event containing merkleRoot. This violates the intended behavior of the contract, which expects merkleRoot to be changed only through the updateDistribution function.

Vulnerability Details

The vulnerability occurs in the pauseForUpdate function, which is marked with the onlyDistributionOracle modifier, allowing the distributionOracle address to call it. When pauseForUpdate is called, it internally calls the _pause() function from the inherited PausableUpgradeable contract. The _pause() function emits a Paused event, which includes the current merkleRoot value in the event data. Emitting an event with merkleRoot is considered a change to the merkleRoot variable, violating the expected behavior.

The reason for using the onlyDistributionOracle modifier can be inferred from the contract's design and the purpose of the pauseForUpdate function:

1. The PriorityPool contract has a state variable called distributionOracle, which is set using the setDistributionOracle function. This suggests that the contract relies on an external oracle to perform certain actions related to token distribution.

2. The pauseForUpdate function is intended to pause the queueing and unqueueing of tokens so that a new merkle tree can be generated. This is evident from the comment above the function: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/priorityPool/PriorityPool.sol#L510-L512

• The onlyDistributionOracle modifier ensures that only the designated distributionOracle address can call the pauseForUpdate function. This implies that the contract trusts the distributionOracle to initiate the process of generating a new merkle tree by pausing the queueing and unqueueing of tokens.

The use of the onlyDistributionOracle modifier suggests that the contract designers intended to give the distributionOracle the authority to control when the contract should be paused for updating the merkle tree. This could be because the distributionOracle is responsible for generating and providing the updated merkle tree data to the contract.

However, allowing the distributionOracle to call pauseForUpdate indirectly modifies the merkleRoot state variable, which violates the expected behavior of the contract. This highlights a potential issue with the contract's design and the trust placed in the distributionOracle.

By calling _pause(), the pauseForUpdate function indirectly modifies the merkleRoot state variable, violating the expected behavior of the contract. The onlyDistributionOracle modifier allows the distributionOracle address to call pauseForUpdate, which leads to the unauthorized modification of merkleRoot.

Impact

The vulnerability allows the distributionOracle to indirectly modify the merkleRoot value, going against the intended behavior of the contract. If external systems or users rely on the Paused event to track changes to merkleRoot, they may make incorrect assumptions about the state of the contract. The inconsistency between the expected and actual behavior of merkleRoot modifications could lead to issues in the contract's functionality and integrations.

Tools Used

Vs Code

Recommendations

1. Remove the onlyDistributionOracle modifier from pauseForUpdate and restrict access to only the contract owner: