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

There Is No Check For Aggregated Amount Deposit in `SablierV2MerkleLL` and `SablierV2MerkleLT` Contract

4rdiii

[M-1] There Is No Check For Aggregated Amount Deposit in SablierV2MerkleLL and SablierV2MerkleLT Contract

Description:

The SablierV2MerkleLockupFactory::createMerkleLL functions does not transmit the aggregateAmount to the newly instantiated MerkleLockup. Consequently, the MerkleLockup contract lacks a mechanism to validate whether the deposited amount aligns with the required aggregateAmount. This oversight, due to manual handling the deposit process, may lead to discrepancies where the airdrop admin deposits an amount either higher or lower than the aggregateAmount.

Impact:
Without this verification, the contract still operates, but several issues may arise:

  • If the deposited amount exceeds the aggregateAmount and the grace period has elapsed, the surplus amount will be irretrievable.

function clawback(address to, uint128 amount) external override onlyAdmin {
@> // Check: current timestamp is over the grace period and the campaign has not expired.
@> if (_hasGracePeriodPassed() && !hasExpired()) {
revert Errors.SablierV2MerkleLockup_ClawbackNotAllowed({
blockTimestamp: block.timestamp,
expiration: EXPIRATION,
firstClaimTime: _firstClaimTime
});
}
​
// Effect: transfer the tokens to the provided address.
ASSET.safeTransfer(to, amount);
​
// Log the clawback.
emit Clawback(admin, to, amount);
}

  • If the deposited amount falls short of the aggregateAmount, some users may be unable to claim. However, this scenario can be rectified by supplementing the deposit with additional tokens.

Proof of Concept:
Given the second scenario's lesser severity and its ease of correction, only the first scenario is illustrated here:

POC

  1. Adjust the funding to ensure the deposited amount surpasses the AGGREGATE_AMOUNT = numberOfRecipients * CLAIM_AMOUNT:

abstract contract MerkleLockup_Integration_Test is Integration_Test {
uint128 public constant CLAIM_AMOUNT = 10_000e18;
​
function setUp() public virtual override {
Integration_Test.setUp();
​
// Create the default MerkleLockup contracts.
merkleLL = createMerkleLL();
merkleLT = createMerkleLT();
​
// Fund the MerkleLockup contracts.
+ deal({ token: address(dai), to: address(merkleLL), give: defaults.AGGREGATE_AMOUNT() + 1 * CLAIM_AMOUNT });
- deal({ token: address(dai), to: address(merkleLL), give: defaults.AGGREGATE_AMOUNT() });
deal({ token: address(dai), to: address(merkleLT), give: defaults.AGGREGATE_AMOUNT() });
}
.
.
.
}

  1. Incorporate the following test into the v2-periphery/test/integration/merkle-lockup/ll/claim/claim.t.sol test suite:

function test_HigherAmountDeposited() public givenCampaignNotExpired givenNotClaimed givenIncludedInMerkleTree {
assertEq(dai.balanceOf(address(merkleLL)), 5 * CLAIM_AMOUNT); // check if higher amount is deposited correct amount is 4* CLAIM_AMOUNT
claimLL(); //start one stream
vm.stopPrank();
vm.warp(block.timestamp + 7 days + 1); // graceback has passed
address to = makeAddr("dummy");
uint128 amount = uint128(dai.balanceOf(address(merkleLL)));
vm.prank(merkleLL.admin());
vm.expectRevert();
merkleLL.clawback(to, amount);
}

Recommended Mitigation:
To address this issue, assets should be transferred in the constructor or an initialization function. Additionally, ensure the correct amount is deposited by transmitting the aggregated amount from the factory contract when creating the MerkleLock contracts.

+ uint256 immutable i_aggregateAmount;
constructor(
MerkleLockup.ConstructorParams memory baseParams,
ISablierV2LockupLinear lockupLinear,
LockupLinear.Durations memory streamDurations_,
+ uint256 _aggregateAmount
) SablierV2MerkleLockup(baseParams) {
+ i_aggregateAmount = _aggregateAmount;
LOCKUP_LINEAR = lockupLinear;
streamDurations = streamDurations_;
+ ASSET.safeTransferFrom(msg.sender,address(this),i_aggregateAmount);
// Max approve the Sablier contract to spend funds from the MerkleLockup contract.
ASSET.forceApprove(address(LOCKUP_LINEAR), type(uint256).max);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Known - Contest Details

https://www.codehawks.com/contests/clvb9njmy00012dqjyaavpl44

Support

FAQs

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