Sablier
Sablier
DeFiFoundry
53,440 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Recipient with index of zero can claim stream multiple times.

iamemptyset

Summary

When a recipients index start from zero the coresponding recipient with index of zero can call SablierV2MerkleLL.sol::claim and SablierV2MerkleLT.sol::claim functions multiple times.

Vulnerability Details

The SablierV2MerkleLL.sol::claim and SablierV2MerkleLT.sol::claim functions called SablierV2MerkleLockup::_checkClaim internal function to check if the recipient index already claimed or not using below code.

if (_claimedBitMap.get(index)) {
revert Errors.SablierV2MerkleLockup_StreamClaimed(index);
}

But when the recipient index is zero it will always return false and allow the recipient to claim again.

Impact

The vulnerability will allow recipient with an index of zero have multiples NFTs (streams) which will allow him to have token amount multiple times.

Proof of Code

In the /v2-periphery/test/utils/Defaults.sol change the INDEX1, INDEX2, INDEX3 and INDEX4 constant variables value to 0, 1, 2, 3 from 1, 2, 3, 4 to make the recipients index start from zero. Then in /v2-periphery/test/integration/merkle-lockup/ll/claim/claim.t.sol test file add the below test code.

function test_ClaimWithZeroIndex() external givenCampaignNotExpired givenNotClaimed givenIncludedInMerkleTree {
for (uint256 i = 0; i < 5; i++) {
uint256 expectedStreamId = lockupLinear.nextStreamId();
​
vm.expectEmit({ emitter: address(merkleLL) });
emit Claim(defaults.INDEX1(), users.recipient1, defaults.CLAIM_AMOUNT(), expectedStreamId);
uint256 actualStreamId = claimLL();
​
LockupLinear.StreamLL memory actualStream = lockupLinear.getStream(actualStreamId);
LockupLinear.StreamLL memory expectedStream = LockupLinear.StreamLL({
amounts: Lockup.Amounts({ deposited: defaults.CLAIM_AMOUNT(), refunded: 0, withdrawn: 0 }),
asset: dai,
cliffTime: getBlockTimestamp() + defaults.CLIFF_DURATION(),
endTime: getBlockTimestamp() + defaults.TOTAL_DURATION(),
isCancelable: defaults.CANCELABLE(),
isDepleted: false,
isStream: true,
isTransferable: defaults.TRANSFERABLE(),
recipient: users.recipient1,
sender: users.admin,
startTime: getBlockTimestamp(),
wasCanceled: false
});
​
assertTrue(merkleLL.hasClaimed(defaults.INDEX1()), "not claimed");
assertEq(actualStreamId, expectedStreamId, "invalid stream id");
assertEq(actualStream, expectedStream);
}
}

Tools Used

Manual review

Recommendations

Prevent SablierV2MerkleLL and SablierV2MerkleLT creators from assigning an index of zero to recipient. Also add the below check in SablierV2MerkleLockup::_checkClaim internal function to enhance security.

require(index > 0);
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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