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

Unrestricted Gas Usage in `SablierV2Lockup::onLockupStreamCanceled` Enables Recipient to Block Stream Cancellation by Sender

pina

Summary

The sender has the option to cancel their own stream for any reason. Once the cancellation is initiated, the streamed money is delivered to the recipient and the sender withdraws the remaining funds. However, before the SablierV2Lockup::cancel transaction is completed, the recipient can deny the completion of this transaction.

Vulnerability Details

In any case where the sender feels uncomfortable with the transaction and wants to rescue the remaining funds, the sender calls the SablierV2Lockup::cancel function. After the initial check is done, SablierV2Lockup::_cancel is called. At that point, the streamed money is delivered to the recipient and the remaining funds are intended to be sent back to the sender. However, before the call is finished, this hook is invoked:

if (recipient.code.length > 0) {
@> try ISablierV2Recipient(recipient).onLockupStreamCanceled({
streamId: streamId,
sender: sender,
senderAmount: senderAmount,
recipientAmount: recipientAmount
}) { } catch { }
}
​

This allows the recipient to control this call and use all the gas, preventing the transaction from completing.

Moreover, the recipient can use an EOA as the recipient when the stream is created and show no intention of malicious behavior. However, after the creation of the stream, when the NFT is minted, it can be transferred to a new recipient with a malicious implementation in the hooks.

Note: This can also affect the SablierV2Lockup::renounce function, but since this call benefits the recipient, it is not included as a severity issue.

PoC

If you implement this minimal test in Remix IDE, you may encounter the error transact to Sablier.withdraw errored: estimated gas for this transaction (113203666) is higher than gasLimit set in the configuration (0x2710). Please raise the gas limit.

​
contract Sablier {
​
MaliciousSender recipient;
​
constructor(address _recipient) {
recipient = MaliciousSender(_recipient);
}
​
function withdraw(uint256 _amount) public {
try recipient.onLockupStreamCanceled({
streamId: 2,
sender: msg.sender,
senderAmount: _amount,
recipientAmount: uint128(_amount)
}) {
​
} catch { }
}
}
​
contract MaliciousSender {
​
function onLockupStreamCanceled(uint256 streamId, address sender, uint256 senderAmount, uint128 recipientAmount) external pure {
uint256 a = 20; uint256 b = 30;uint256 c = 40; uint256 d = 10;uint256 e = 2;uint256 f = 60;uint256 x = 2;uint256 y = 10;uint256 z = 100;uint256 result;
​
while (true) {
result = (((((((x ** y) % z) + a) * b) / c) - d) ** e) % f;
}
}
}

Impact

The recipient's can prevent the sender from canceling its stream.

Tools Used

  • Manual code review

Recommendations

Set a specific gasLimitRecipient that the sender can use, such as:

if (recipient.code.length > 0) {
+ try ISablierV2Recipient(recipient).onLockupStreamCanceled{gas: gasLimitRecipient }({
- try ISablierV2Recipient(recipient).onLockupStreamCanceled({
streamId: streamId,
sender: sender,
senderAmount: senderAmount,
recipientAmount: recipientAmount
}) { } catch { }
}

This way, the sender can perform their functionality without denying the cancellation. Test the complexity to estimate the recipient's needs.

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.