Vyper Vested Claims
First Flight #34
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Merkle Root Manipulation Risk: Preventing Permanent Loss of Vested Token Claims

betrue

Summary

Current implementation of setting merkle root is unsafe because it allows owner to change the root any time (before the vesting duration is over) which could potentially invalidate in-progress claims.

Vulnerability Details

In the VestedAirdrop::set_merkle_root function, the owner can change the merkle root at any time:

def set_merkle_root(merkle_root: bytes32):
"""
@notice This function is used to set the merkle root
@param merkle_root bytes32, the new merkle root
@dev This function can only be called by the owner
"""
self.onlyOwner()
self.merkle_root = merkle_root
log MerkleRootUpdated(merkle_root)

With this implementation, there is no mechanism to prevent malicious or erroneous updates to the merkle root which may invalidate legitimate claims and allow unauthorized claims.

Proof of Concept

Here is a test that presents a case where updating the merkle root can invalidate in-progress claims:

def test_merkle_root_update_vulnerability(self):
"""
Demonstrates how updating merkle root can invalidate legitimate claims
"""
​
# User1 claims their initial TGE portion (31%)
self.airdrop.claim(self.user1, self.amount, self.proof, sender=self.user1)
initial_claim = self.token.balanceOf(self.user1)
assert initial_claim >= (self.amount * 31) // 100
​
# Owner maliciously updates merkle root
malicious_root = bytes.fromhex(
"1234567890123456789012345678901234567890123456789012345678901234"
)
# Get the owner from deployment
owner = self.airdrop.owner()
with boa.env.prank(owner):
self.airdrop.set_merkle_root(malicious_root)
​
# Fast forward to after vesting period
vesting_end = self.airdrop.vesting_end_time() # Get end time from contract
warp(vesting_end + 1)
​
# User1 cannot claim their remaining tokens (69%) because proof invalid
with boa.reverts("Invalid proof"):
self.airdrop.claim(self.user1, self.amount, self.proof, sender=self.user1)
​
# User1's balance remains stuck at initial claim
final_balance = self.token.balanceOf(self.user1)
assert final_balance == initial_claim
assert (
final_balance < self.amount
) # Should have gotten full amount after vesting

Impact

While there are legitimate reasons for having the functionality to be able to reset the merkle root (such as contract resuability) the current implementation has no protection for already claimed amounts and has no timelock or governance controls. A premature or malicious reset of the merkle root can cause users to lose their existing proofs and as a result they cannot claim their remaining tokens.

Tools Used

Manual inspection, copilot and moccasin.

Recommendations

Implementing a timelock or "no updates after claims start" mechanism that respects the vesting duration would make the contract more secure:

# ...existing code...
​
# Add state variables
merkle_update_timelock: public(uint256)
TIMELOCK_DELAY: constant(uint256) = 7 * 24 * 60 * 60 # e.g. 7 days or some other suitable choice
proposed_merkle_root: public(bytes32)
has_claims: public(bool)
​
@external
def propose_merkle_root(new_root: bytes32):
"""
@notice Propose a new merkle root with timelock
"""
self.onlyOwner()
assert not self.has_claims, "Cannot update after claims started"
self.proposed_merkle_root = new_root
self.merkle_update_timelock = block.timestamp + TIMELOCK_DELAY
​
@external
def execute_merkle_root_update():
"""
@notice Execute the proposed merkle root update after timelock
"""
self.onlyOwner()
assert block.timestamp >= self.merkle_update_timelock, "Timelock not expired"
assert self.proposed_merkle_root != empty(bytes32), "No proposed root"
assert not self.has_claims, "Cannot update after claims started"
​
self.merkle_root = self.proposed_merkle_root
self.proposed_merkle_root = empty(bytes32)
log MerkleRootUpdated(self.merkle_root)
​
@external
def claim(user: address, total_amount: uint256, proof: DynArray[bytes32, 20]) -> bool:
# ...existing claim code...
self.has_claims = True # Set on first claim
# ...rest of existing code...

This improved version requires a timelock delay, prevents premature updates and still allows contract reuse for new distribution phases. Additional protection where partial-claim history can be stored and verified could be a nice improvement so that a root update does not wipe out users' partial progress.

Updates

Appeal created

bube Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[Invalid] No checks in `set_merkle_root` function

The `set_merkle_root` function is called only by the `owner` and the `owner` is trusted. This means the input argument `merkle_root` will be correct and the `owner` will not call again the `set_merkle_root` function.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.