sherlock-audit / 2024-05-tokensoft-distributor-contracts-update-judging

3 stars 2 forks source link

0xboriskataa - Incorrect merkle proof check used for tranche vesting #50

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

0xboriskataa

medium

Incorrect merkle proof check used for tranche vesting

Summary

Incorrect merkle proof check used for tranche vesting contracts

Vulnerability Detail

The files in scope that have "Merkle" in the file name use the modifier validMerkleProof(bytes32 leaf, bytes32[] memory merkleProof) to validate data inputed by the user which is specific depending on the type of contract.

For example PerAddressContinuousVestingMerkle.sol validates start, cliff and end which are specific attributes related to continuous vesting:

validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, amount, start, cliff, end)), merkleProof)

Another example is PerAddressTrancheVestingMerkle.sol in which tranches are validated instead:

validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, amount, abi.encode(tranches))), merkleProof)

The issues is that PerAddressTrancheVestingMerkleDistributor.sol doesn't validate any tranches. Instead it just validates generic data:

validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, amount)), merkleProof)

We can compare it to PerAddressContinuousVestingMerkleDistributor.sol :

validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, amount, start, cliff, end)), merkleProof)

As you can see the continuous vesting distributor still validates start, cliff and end like PerAddressContinuousVestingMerkle.sol.

Impact

This allows the user to use less data and still pass the modifier check. He can for example call claim() and claim his tokens without specifying any tranches as leaves.

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/factory/PerAddressTrancheVestingMerkleDistributor.sol#L47

Tool used

Manual Review

Recommendation

Change the inputs for the modifier to this:

- validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, amount)), merkleProof)
+ validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, amount, abi.encode(tranches))), merkleProof)
MxAxM commented 3 months ago

despite mentioning the root cause, it doesn't describe the impact clearly.