cryptic - `EscrowedEXA::onStreamCanceled` callback will never execute due to incorrect recipient address

[sherlock-admin3]

cryptic

medium

EscrowedEXA::onStreamCanceled callback will never execute due to incorrect recipient address

Summary

EscrowedEXA contains functionality for vesting EXA tokens, and allows the recipient to cancel the vesting stream. When cancelling a vesting steam, EscrowedEXA contract implements onStreamCanceled callback to mint esEXA to the recipient with the remaining EXA received from the canceled stream.

The problem is that this callback will always fail silently due to incorrect recipient address, and the caller will not receive the remaining esEXA tokens.

Vulnerability Detail

When cancelling a vesting stream the following function is initiated:

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

  function cancel(uint256[] memory streamIds) external returns (uint256 streamsReserves) {
    uint128 refundableAmount;
    for (uint256 i = 0; i < streamIds.length; ++i) {
      uint256 streamId = streamIds[i];
      checkStream(streamId);
      assert(msg.sender == sablier.getRecipient(streamId));
      streamsReserves += reserves[streamId];
      delete reserves[streamId];
      refundableAmount += sablier.refundableAmountOf(streamId);
      withdrawMax(streamId);
@>    sablier.cancel(streamId);
    }
    emit Cancel(msg.sender, streamIds);
    _mint(msg.sender, refundableAmount);
    exa.safeTransfer(msg.sender, streamsReserves);
  }

When sablier.cancel(streamId) is called, the stream is cancelled via the Sablier external contract, which initiates the onStreamCanceled callback. This callback is optional and will fail silently if not implemented, as stated in the Sablier docs:

ISablierV2LockupRecipient https://etherscan.io/address/0xafb979d9afad1ad27c5eff4e27226e3ab9e5dcc9#code

interface ISablierV2LockupRecipient {
    /// @dev Notes:
    /// - This function may revert, but the Sablier contract will ignore the revert.
    function onStreamCanceled(
        uint256 streamId,
        address sender,
        uint128 senderAmount,
        uint128 recipientAmount
    )
        external;
}

EscrowedEXA contract implements this function to mint esEXA to the recipient with the remaining EXA received from the cancelled stream.

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

  /// @notice Hook called when a recipient cancels a stream.
  /// @notice Mints esEXA to the recipient with the remaining EXA received from the canceled stream.
  /// @param streamId streamId of the cancelled stream.
  /// @param recipient recipient of the cancelled stream.
  /// @param senderAmount amount of EXA received back from the stream cancelling.
  function onStreamCanceled(uint256 streamId, address recipient, uint128 senderAmount, uint128) external {
    assert(msg.sender == address(sablier));
    checkStream(streamId);
    _mint(recipient, senderAmount);
    returnReserve(streamId, recipient);
  }

However, there is an issue here. Let's see what happens when a vesting stream is created:

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

  function vest(uint128 amount, address to, uint256 maxRatio, uint256 maxPeriod) public returns (uint256 streamId) {
    .
    .
    .

    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 })
      })
    );
    .
    .
    .
  }

Note that sender = address of EscrowedEXA contract and recipient is the to address passed in.

When we cancel via sablier.cancel(streamId), the external function is called:

SablierV2LockupLinear::_cancel https://etherscan.io/address/0xafb979d9afad1ad27c5eff4e27226e3ab9e5dcc9#code

    function _cancel(uint256 streamId) internal override {
       .
       .
       .

        // @audit sender is address of `EscrowedEXA` contract
        address sender = _streams[streamId].sender;
        // @audit recipient is address of recipient cancelling the stream
        address recipient = _ownerOf(streamId);

          // @audit recipient address is the address of `recipient` defined in `createWithDurations`
         // It is NOT the address of the `EscrowedEXA` contract
         if (recipient.code.length > 0) {
            try ISablierV2LockupRecipient(recipient).onStreamCanceled({
                streamId: streamId,
                sender: sender,
                senderAmount: senderAmount,
                recipientAmount: recipientAmount
            }) { } catch { }
        }

    }

As we can see, sender is the address of the EscrowedEXA contract, however ISablierV2LockupRecipient(recipient).onStreamCanceled is called on the recipient address which is the initial to address from when the vesting stream was created. Therefore, the onStreamCanceled of the EscrowedEXA contract will not be executed, and due to the try/catch block, it will fail silently.

Impact

onStreamCanceled callback will silently fail, recipient will not receive the esEXA tokens from the remaining EXA received by cancelling the stream as intended by the callback.

Code Snippet

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

Tool used

Manual Review

Recommendation

Perhaps redesign how the sender and recipient addresses are set upon creating new vesting streams, but that may also have some implications, so be very careful when implementing the solution.

[santipu03]

This issue is invalid because the callback onStreamCanceled will only be called if the stream was canceled directly through Sablier. When a user calls EscrowedEXA::cancel, it will receive the exEXA tokens here.