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

3 stars 2 forks source link

Ironsidesec - AdvancedDistributorInitializable is sending 0 bytes to decode which causes DOS #32

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

Ironsidesec

medium

AdvancedDistributorInitializable is sending 0 bytes to decode which causes DOS

Summary

Impact : PerAddressContinuousVestingMerkleDistributor and PerAddressTrancheVestingMerkleDistributor vesting beneficiaries can never claim their tokens. Even if data is passed from claim(),  the internal call AdvancedDistributorInitializable._executeClaim makes them 0.

Vulnerability Detail

Claiming from PerAddressContinuousVestingMerkleDistributor and PerAddressTrancheVestingMerkleDistributor contracts is not possible due to two reasons

  1. In PerAddressContinuousVestingMerkleDistributor.claim() itself, the data is sent zero in L67 below, so it will revert on L38 below on PerAddressTrancheVestingInitializable.getVestedFraction()

  2. But even if it fixed, the L38 below reverts due to L113 below on AdvancedDistributorInitializable._executeClaim, because AdvancedDistributorInitializable is inherited by PerAddressContinuousVestingMerkleDistributor.

So fixing 1 doesn't fix the other or vice versa.

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/factory/PerAddressTrancheVestingInitializable.sol#L38

File: 2024-05-tokensoft-distributor-contracts-update\contracts\packages\hardhat\contracts\claim\factory\PerAddressTrancheVestingInitializable.sol

37:     function getVestedFraction(address beneficiary, uint256 time, bytes memory data) public view override returns (uint256) {
38:  >>>    Tranche[] memory tranches = abi.decode(data, (Tranche[]));
39: 
40:         uint256 delay = getFairDelayTime(beneficiary);
... SNIP ...
52:     }

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

File: 2024-05-tokensoft-distributor-contracts-update\contracts\packages\hardhat\contracts\claim\factory\PerAddressContinuousVestingMerkleDistributor.sol

52:     function claim(
... SNIP ...
60:     )
61:         external
62:         validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, totalAmount, start, cliff, end)), merkleProof)
63:         nonReentrant
64:     {
65:         // effects
67:   >>>   uint256 claimedAmount = super._executeClaim(beneficiary, totalAmount, new bytes(0));
... SNIP ...
70:     }

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/factory/AdvancedDistributorInitializable.sol#L112

2024-05-tokensoft-distributor-contracts-update\contracts\packages\hardhat\contracts\claim\factory\AdvancedDistributorInitializable.sol

106:     function _executeClaim(address beneficiary, uint256 totalAmount, bytes memory)
110:         returns (uint256 _claimed)
111:     {
113:   >>>   _claimed = super._executeClaim(beneficiary, totalAmount, new bytes(0)); 
114:         _reconcileVotingPower(beneficiary);
115:     }

Impact

PerAddressContinuousVestingMerkleDistributor and PerAddressTrancheVestingMerkleDistributor vesting beneficiaries can never claim their tokens. Even if data is passed from claim(),  the internal call AdvancedDistributorInitializable._executeClaim makes them 0.

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/factory/AdvancedDistributorInitializable.sol#L112

Tool used

Manual Review

Recommendation

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/67edd899612619b0acefdcb0783ef7a8a75caeac/contracts/packages/hardhat/contracts/claim/factory/AdvancedDistributorInitializable.sol#L112

-   function _executeClaim(address beneficiary, uint256 totalAmount, bytes memory)
+   function _executeClaim(address beneficiary, uint256 totalAmount, bytes memory data)
 internal
 virtual
 override
 returns (uint256 _claimed)
 {
-       _claimed = super._executeClaim(beneficiary, totalAmount, new bytes(0)); 
+       _claimed = super._executeClaim(beneficiary, totalAmount, data); 
 _reconcileVotingPower(beneficiary);
 }

Duplicate of #11