Former recipient loses part of deposited reserve that is proportional to her or his vested time after withdrawing from corresponding vesting stream and transferring it to another recipient before it's fully vested

Summary

The vesting streams created by the EscrowedEXA.vest function are transferable. However, if the recipient withdraws from the vesting stream and transfers it to another recipient before it's fully vested, the former recipient loses the part of the deposited reserve that is proportional to her or his vested time because the EscrowedEXA.withdrawMax function does not transfer the part of the deposited reserve, which the former recipient should rightfully receive, to the former recipient.

Vulnerability Detail

Since the vest function shown in the Code Snippet section sets the created vesting stream's transferable to true, the corresponding vesting stream is transferable. For any time after the vesting stream is started and before it is fully vested, sablier.isDepleted(streamId) would be false for the corresponding vesting stream; in this case, when the recipient of such vesting stream withdraws from it and transfers it to another recipient, the former recipient cannot receive any of the deposited reserve because the withdrawMax function shown in the Code Snippet section executes if (sablier.isDepleted(streamId)) returnReserve(streamId, msg.sender). Thus, even if the former recipient has vested the vesting streaming until one second before it is fully vested, withdrawing from it and transferring it to another recipient will cause the former recipient to receive none of the deposited reserve even though the former recipient has vested for the majority of the vesting period and should be entitled to the part of the deposited reserve that is proportional to her or his vested time.

For POC, please add the following test in protocol\test\EscrowedEXA.t.sol. This test will pass to demonstrate the described scenario.

  function test_formerRecipientReceivesNoneOfReserveAfterWithdrawingFromAndTransferringStream() external {
    address CHARLIE = address(123);
    address BOB = address(321);

    uint256 amount = 1_000 ether;
    uint256 ratio = esEXA.reserveRatio();
    uint256 reserve = amount.mulWadDown(ratio);
    esEXA.mint(amount, address(this));

    // create vesting stream to Charlie
    uint256 streamId = esEXA.vest(uint128(amount), CHARLIE, ratio, esEXA.vestingPeriod());
    uint256[] memory streamIds = new uint256[](1);
    streamIds[0] = streamId;

    // some reserve has been deposited after vest function call
    uint256 reserveAfterVestFnCall = esEXA.reserves(streamId);
    assertGt(reserveAfterVestFnCall, 0);

    // just before vesting stream is fully vested, Charlie has no EXA tokens at that moment
    vm.warp(block.timestamp + esEXA.vestingPeriod() - 1);
    assertEq(exa.balanceOf(CHARLIE), 0);

    uint256 withdrawableAmountCharlie = ISablierV2Lockup(address(sablier)).withdrawableAmountOf(streamId);

    vm.startPrank(CHARLIE);

    esEXA.withdrawMax(streamIds);

    // after withdrawing from vesting stream, Charlie owns his withdrawable amount
    assertEq(exa.balanceOf(CHARLIE), withdrawableAmountCharlie);

    // but Charlie receives none of deposited reserve even though he has vested for majority of vesting period
    uint256 reserveAfterWithdrawalCharlie = esEXA.reserves(streamId);
    assertEq(reserveAfterWithdrawalCharlie, reserveAfterVestFnCall);

    // Charlie transfers vesting stream to Bob before it is fully vested
    IERC721(address(sablier)).transferFrom(CHARLIE, BOB, streamId);

    vm.stopPrank();

    // when vesting stream is fully vested, Bob has no EXA tokens at that moment
    vm.warp(block.timestamp + esEXA.vestingPeriod());
    assertEq(exa.balanceOf(BOB), 0);

    uint256 withdrawableAmountBob = ISablierV2Lockup(address(sablier)).withdrawableAmountOf(streamId);

    vm.startPrank(BOB);

    esEXA.withdrawMax(streamIds);

    vm.stopPrank();

    // after withdrawing from vesting stream, Bob owns his withdrawable amount and all of deposited reserve
    assertEq(exa.balanceOf(BOB), reserveAfterVestFnCall + withdrawableAmountBob);

    // all of deposited reserve has been transferred to Bob while Charlie has received none of it
    uint256 reserveAfterWithdrawalBob = esEXA.reserves(streamId);
    assertEq(reserveAfterWithdrawalBob, 0);
  }

Impact

After withdrawing from the vesting stream and transferring it to another recipient, the former recipient fails to receive the part of the deposited reserve that is proportional to her or his vested time comparing to the vesting period, which should be entitled to the former recipient.

Code Snippet

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/periphery/EscrowedEXA.sol#L89-L110

  function vest(uint128 amount, address to, uint256 maxRatio, uint256 maxPeriod) public returns (uint256 streamId) {
    ...
    uint256 reserve = amount.mulWadUp(reserveRatio);
    exa.safeTransferFrom(msg.sender, address(this), reserve);
    streamId = sablier.createWithDurations(
      CreateWithDurations({
        asset: exa,
        sender: address(this),
        recipient: to,
        totalAmount: amount,
        cancelable: true,
        transferable: true,
        durations: Durations({ cliff: 0, total: vestingPeriod }),
        broker: Broker({ account: address(0), fee: 0 })
      })
    );
    reserves[streamId] = reserve;
    ...
  }

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/periphery/EscrowedEXA.sol#L163-L166

  function withdrawMax(uint256 streamId) internal {
    if (sablier.withdrawableAmountOf(streamId) != 0) sablier.withdrawMax(streamId, msg.sender);
    if (sablier.isDepleted(streamId)) returnReserve(streamId, msg.sender);
  }

Tool used

Manual Review

Recommendation

The withdrawMax function can be updated to distribute the part of the deposited reserve that is proportional to the recipient's vested time comparing to the vesting period to the recipient instead of distributing all of it when sablier.isDepleted(streamId) is true. Then, for the same vesting stream, the remaining part(s) of the deposited reserve can be distributed to the corresponding recipient(s) through the subsequent withdrawMax function calls.

sherlock-audit / 2024-04-interest-rate-model-judging

rbserver - Former recipient loses part of deposited reserve that is proportional to her or his vested time after withdrawing from corresponding vesting stream and transferring it to another recipient before it's fully vested #184