Omniscia Stakewise Audit

MerkleDistributor Manual Review Findings

MerkleDistributor Manual Review Findings

MDR-01M: Improper Pause State Access Control

Description:

The claim function's invocation is prohibited when the contract is paused, however, the administrator can still mistakenly distribute tokens in such a state via the distributePeriodically and distributeOneTime functions.

Example:

contracts/merkles/MerkleDistributor.sol
63/**
64 * @dev See {IMerkleDistributor-distributePeriodically}.
65 */
66function distributePeriodically(
67    address from,
68    address token,
69    address beneficiary,
70    uint256 amount,
71    uint256 durationInBlocks
72)
73    external override onlyAdmin
74{
75    require(amount > 0, "MerkleDistributor: invalid amount");
76

77    uint256 startBlock = block.number;
78    uint256 endBlock = startBlock + durationInBlocks;
79    require(endBlock > startBlock, "MerkleDistributor: invalid blocks duration");
80

81    IERC20Upgradeable(token).safeTransferFrom(from, address(this), amount);
82    emit PeriodicDistributionAdded(from, token, beneficiary, amount, startBlock, endBlock);
83}
84

85/**
86 * @dev See {IMerkleDistributor-distributeOneTime}.
87 */
88function distributeOneTime(
89    address from,
90    address origin,
91    address token,
92    uint256 amount,
93    string memory rewardsLink
94)
95    external override onlyAdmin
96{
97    require(amount > 0, "MerkleDistributor: invalid amount");
98

99    IERC20Upgradeable(token).safeTransferFrom(from, address(this), amount);
100    emit OneTimeDistributionAdded(from, origin, token, amount, rewardsLink);
101}

Recommendation:

We advise the whenNotPaused modifier to be added to both functions as well, preventing distributions during a pause state.

Alleviation:

The whenNotPaused modifier was properly introduced to both instances.

MDR-02M: Ineffectual Duration Argument

Description:

The durationInBlocks argument of the distributePeriodically function is meant to signify the block duration in which the rewards are meant to be distributed, however, no such limitation is placed anywhere in the codebase meaning that the start and end block thresholds are not enforced.

Example:

contracts/merkles/MerkleDistributor.sol
63/**
64 * @dev See {IMerkleDistributor-distributePeriodically}.
65 */
66function distributePeriodically(
67    address from,
68    address token,
69    address beneficiary,
70    uint256 amount,
71    uint256 durationInBlocks
72)
73    external override onlyAdmin
74{
75    require(amount > 0, "MerkleDistributor: invalid amount");
76

77    uint256 startBlock = block.number;
78    uint256 endBlock = startBlock + durationInBlocks;
79    require(endBlock > startBlock, "MerkleDistributor: invalid blocks duration");
80

81    IERC20Upgradeable(token).safeTransferFrom(from, address(this), amount);
82    emit PeriodicDistributionAdded(from, token, beneficiary, amount, startBlock, endBlock);
83}

Recommendation:

We advise either the thresholds to be actively enforced in the claim function by repurposing the lastUpdateBlockNumber variable or the arguments to be omitted entirely as in the current implementation they waste gas while being ineffectual.

Alleviation:

The Stakewise team has responded specifying that this is a necessary argument given that it "...is used by the off-chain oracles to periodically calculate the earned rewards". As a result, we consider this exhibit dealt with.

MDR-03M: Potentially Unclaimed Rewards

Description:

The setMerkleRoot does not perform any validation on the _claimedBitMap, meaning that the previously set merkle root may have had even zero claims made on it.

Example:

contracts/merkles/MerkleDistributor.sol
53/**
54 * @dev See {IMerkleDistributor-setMerkleRoot}.
55 */
56function setMerkleRoot(bytes32 newMerkleRoot, string calldata newMerkleProofs) external override {
57    require(msg.sender == address(oracles), "MerkleDistributor: access denied");
58    merkleRoot = newMerkleRoot;
59    lastUpdateBlockNumber = block.number;
60    emit MerkleRootUpdated(msg.sender, newMerkleRoot, newMerkleProofs);
61}

Recommendation:

We advise some form of claim validation to occur, whereby the _claimedBitMap is equal to a particular bit sequence as claims can be made on the behalf of other users. Alternatively, if claims on behalf of other users are undesirable this exhibit can be considered null.

Alleviation:

The Stakewise team has stated that should the user not claim their rewards, they will be included in the next Merkle root update thus considering this exhibit null.

MDR-04M: Potential Claim Race Condition

TypeSeverityLocation
Logical FaultInformationalMerkleDistributor.sol:L128, L143, L144

Description:

The claim function relies entirely on its arguments to assess the validity of a claim. As a result, it is possible for an arbitrary user to inspect the mempool of the blockchain to replicate the exact same arguments as another pending transaction and make the claim on their behalf. While the funds will still be transferred to the intended recipient, this can cause a user experience misbehaviour as the user would see their transaction fail as already claimed whilst it may have been properly processed.

Example:

contracts/merkles/MerkleDistributor.sol
126function claim(
127    uint256 index,
128    address account,
129    address[] calldata tokens,
130    uint256[] calldata amounts,
131    bytes32[] calldata merkleProof
132)
133    external override whenNotPaused
134{
135    address _rewardEthToken = rewardEthToken; // gas savings
136    require(
137        IRewardEthToken(_rewardEthToken).lastUpdateBlockNumber() < lastUpdateBlockNumber,
138        "MerkleDistributor: merkle root updating"
139    );
140

141    // verify the merkle proof
142    bytes32 _merkleRoot = merkleRoot; // gas savings
143    bytes32 node = keccak256(abi.encode(index, tokens, account, amounts));
144    require(MerkleProofUpgradeable.verify(merkleProof, _merkleRoot, node), "MerkleDistributor: invalid proof");
145

146    // mark index claimed
147    _setClaimed(index, _merkleRoot);

Recommendation:

We advise the account member to be validated to be equal to the msg.sender to prevent this condition from arising. If claims on another user's behalf are desired this exhibit can be safely ignored.

Alleviation:

The Stakewise team confirmed that claims on another user's behalf are desired rendering this exhibit null.

Prev

ERC20PermitUpgradeable.sol (ERP-M)

Next

Oracles.sol (ORA-M)

On this page