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

Zero Value Transfer Reverts in SablierV2 Contracts

albahaca

Summary

In the SablierV2 contracts, several instances of ERC20 token transfers do not account for zero value transfers, which can cause transactions to revert unexpectedly. This report identifies the affected lines in different contract files and suggests best practices to handle zero value transfers safely.

Vulnerability Details

The issue arises due to the use of safeTransferFrom and safeTransfer functions in several contract files without checking if the transfer amount is zero. According to the ERC20 standard, while some implementations allow zero value transfers, others may revert. The following lines in the SablierV2 contracts are affected:

File: v2-core/src/SablierV2LockupDynamic.sol
357 params.asset.safeTransferFrom({ from: msg.sender, to: address(this), value: createAmounts.deposit });
361 params.asset.safeTransferFrom({ from: msg.sender, to: params.broker.account, value: createAmounts.brokerFee });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L357
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupDynamic.sol#L361

File: v2-core/src/SablierV2LockupLinear.sol
277 params.asset.safeTransferFrom({ from: msg.sender, to: address(this), value: createAmounts.deposit });
281 params.asset.safeTransferFrom({ from: msg.sender, to: params.broker.account, value: createAmounts.brokerFee });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L277
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupLinear.sol#L281

File: v2-core/src/SablierV2LockupTranched.sol
261 params.asset.safeTransferFrom({ from: msg.sender, to: address(this), value: createAmounts.deposit });
265 params.asset.safeTransferFrom({ from: msg.sender, to: params.broker.account, value: createAmounts.brokerFee });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/SablierV2LockupTranched.sol#L261-L265

File: v2-core/src/abstracts/SablierV2Lockup.sol
599 asset.safeTransfer({ to: sender, value: senderAmount });
652 asset.safeTransfer({ to: to, value: amount });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L599
https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-core/src/abstracts/SablierV2Lockup.sol#L652

File: v2-periphery/src/SablierV2BatchLockup.sol
341 asset.safeTransferFrom({ from: msg.sender, to: address(this), value: amount });

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/SablierV2BatchLockup.sol#L341

File: v2-periphery/src/abstracts/SablierV2MerkleLockup.sol
118 ASSET.safeTransfer(to, amount);

https://github.com/Cyfrin/2024-05-Sablier/tree/main/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L188

Impact

The inability to handle zero value transfers can cause legitimate transactions to fail, leading to disruptions in the functionality of the protocol . This can affect user experience and the reliability of the contracts, potentially causing a loss of confidence in the platform.

Tools Used

  • Manual code review

Recommendations

  1. Implement Zero Value Checks: Before calling safeTransferFrom or safeTransfer, check if the value is zero and handle it appropriately. For example:

    if (amount > 0) {
    asset.safeTransfer(to, amount);
    }
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Info/Gas/Invalid as per Docs

https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

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