search

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

2 stars 1 forks source link

nfmelendez - Many distributor creation transaction with valid merkle root will revert because of a downcasting #23

Closed sherlock-admin3 closed 4 weeks ago

sherlock-admin3 commented 1 month ago

nfmelendez

medium

Many distributor creation transaction with valid merkle root will revert because of a downcasting

Summary

A downcasting from uint256 to uint160 makes many creation of distributions using a valid merkle root to revert.

Vulnerability Detail

Solidity truncates the higher-order bits when downcast, it doesn't fail or revert. In order to create a salt for the FairQueue to create a pseudorandom value the bytes of the merkle root is processed as a downcast uint160(uint256(byte32)) that truncates the higher-order bits but FairQueue when creates the pseudorandom number it reverts if the salt is 0 with message "I demand more randomness". So there are many merkle roots that will fail when creating a distribution, every merkle root that has the pattern 0xffffffffffffffffffffffff0000000000000000000000000000000000000000 where ffffffffffffffffffffffff can be any hex combination when is downcasted to uint160 will be 0 and therefore the transaction will revert.

Affected Distributions:

POC

➜ uint256 merkleroot = 0xffffffffffffffffffffffff0000000000000000000000000000000000000000
➜ uint160 salt = uint160(merkleroot);
➜ salt
Type: uint160
├ Hex: 0x
├ Hex (full word): 0x0
└ Decimal: 0

Impact

All transactions that creates a merkle distribution with valid merkleroot that match the mentioned pattern will revert with message: "I demand more randomness"

Code Snippet

Example of downcasting:

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

Revert code:

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/utilities/FairQueue.sol#L38

Tool used

Manual Review

Recommendation

I would not downcast and accept all possible merkle roots using a uint256 instead of a uint160 to create the pseudo random number

  function _setPseudorandomValue(uint160 salt) internal { // @audit modify this to uint256 salt and generate a psudo random number without overflowing
    if (distancePerSecond == 0) {
      return;
    }
    require(salt > 0, 'I demand more randomness');
    randomValue = uint160(uint256(blockhash(block.number - 1))) ^ salt;
  }