BiasedMerc - PerAddressContinuousVestingMerkle::claim will revert due to bytes(0) data being passed to _executeClaim

[sherlock-admin3]

BiasedMerc

medium

PerAddressContinuousVestingMerkle::claim will revert due to bytes(0) data being passed to _executeClaim

Summary

PerAddressContinuousVestingMerkle::claim passes bytes(0) to PerAddressContinuousVesting::getVestedFraction which then attempts to decode this into 3 uint256 which will revert due to type mismatch.

Vulnerability Detail

PerAddressContinuousVestingMerkle::claim()

  function claim(
    uint256 index, // the beneficiary's index in the merkle root
    address beneficiary, // the address that will receive tokens
    uint256 totalAmount, // the total claimable by this beneficiary
    uint256 start, // the start of the vesting period
    uint256 cliff, // cliff time
    uint256 end, // the end of the vesting period
    bytes32[] calldata merkleProof
  )
    external
    validMerkleProof(keccak256(abi.encodePacked(index, beneficiary, totalAmount, start, cliff, end)), merkleProof)
    nonReentrant
  {
    // effects
>   uint256 claimedAmount = _executeClaim(beneficiary, totalAmount, new bytes(0));

_executeClaim() is called by passing new bytes(0) as the data parameter, this will lead to a revert further in the call. Lets analyse the flow of PerAddressContinuousVestingMerkle::claim:

1. PerAddressContinuousVestingMerkle::claim()
2. AdvancedDistributor::_executeClaim()
3. Distributor::_executeClaim()
4. Distributor::getClaimableAmount()
5. PerAddressContinuousVesting::getVestedFraction()

    function getVestedFraction(
        address beneficiary,
        uint256 time, // time is in seconds past the epoch (e.g. block.timestamp)
    bytes memory data
    ) public view override returns (uint256) {
        (uint256 start, uint256 cliff, uint256 end) = abi.decode(data, (uint256, uint256, uint256));

When PerAddressContinuousVesting::getVestedFraction() is called, data is still new bytes(0) and there is an attempt to decode it into 3 uint256. This will lead to a revert as the passed bytes data is not decodable into this format.

POC

During the contest there were issues with the testing suite, therefore I have created a very simple test to confirm this behaviour that can be run in REMIX:

pragma solidity 0.8.0;

contract data {
    function test() public pure returns (uint256, uint256, uint256) {
        bytes memory a = new bytes(0);
        (uint256 start, uint256 cliff, uint256 end) = abi.decode(a, (uint256, uint256, uint256));
        return (start, cliff, end);
    }
}

Running the data::test() in remix demonstrates how the call will revert due to the use of new bytes(0) and trying to decode it to 3 integers.

Impact

PerAddressContinuousVestingMerkle::claim will revert and break functionality of the code.

Code Snippet

Tool used

Manual Review

Recommendation

Either pass an appropriate data parameter that can be decoded to 3 uint256 or add a default case to PerAddressContinuousVesting::getVestedFraction() that will be triggered if the passed data parameter is bytes(0)

Duplicate of #11