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

Some tokens revert on zero value approval

bube

Summary

The protocol uses forceApprove function to handle the case when the asset requires firstly to be approved to zero. But there are also ERC-20 tokens that revert if they are approved to zero value.

Vulnerability Details

The contracts SablierV2MerkleLT, SablierV2MerkleLT and SablierV2BatchLockup use the forceApprove function to approve the amount of required asset and to handle the case when the asset is USDT and this type of tokens reverts when the approve is not firstly set to 0.

SablierV2MerkleLT:

constructor(
MerkleLockup.ConstructorParams memory baseParams,
ISablierV2LockupTranched lockupTranched,
MerkleLT.TrancheWithPercentage[] memory tranchesWithPercentages
)
SablierV2MerkleLockup(baseParams)
{
LOCKUP_TRANCHED = lockupTranched;
​
// Since Solidity lacks a syntax for copying arrays of structs directly from memory to storage, a manual
// approach is necessary. See https://github.com/ethereum/solidity/issues/12783.
uint256 count = tranchesWithPercentages.length;
for (uint256 i = 0; i < count; ++i) {
_tranchesWithPercentages.push(tranchesWithPercentages[i]);
}
​
// Max approve the Sablier contract to spend funds from the MerkleLockup contract.
@> ASSET.forceApprove(address(LOCKUP_TRANCHED), type(uint256).max);
}
​

SablierV2MerkleLL:

​
constructor(
MerkleLockup.ConstructorParams memory baseParams,
ISablierV2LockupLinear lockupLinear,
LockupLinear.Durations memory streamDurations_
)
SablierV2MerkleLockup(baseParams)
{
LOCKUP_LINEAR = lockupLinear;
streamDurations = streamDurations_;
​
// Max approve the Sablier contract to spend funds from the MerkleLockup contract.
@> ASSET.forceApprove(address(LOCKUP_LINEAR), type(uint256).max);
}
​

SablierV2BatchLockup:

​
/// @dev Helper function to approve a Sablier contract to spend funds from the batchLockup. If the current allowance
/// is insufficient, this function approves Sablier to spend the exact `amount`.
/// The {SafeERC20.forceApprove} function is used to handle special ERC-20 assets (e.g. USDT) that require the
/// current allowance to be zero before setting it to a non-zero value.
function _approve(address sablierContract, IERC20 asset, uint256 amount) internal {
uint256 allowance = asset.allowance({ owner: address(this), spender: sablierContract });
if (allowance < amount) {
@> asset.forceApprove({ spender: sablierContract, value: amount });
}
}
​

But there is also another problem. There are some ERC-20 assets that revert when the approval is 0:

Some tokens (e.g. BNB) revert when approving a zero value amount.

https://github.com/d-xo/weird-erc20?tab=readme-ov-file#revert-on-zero-value-approvals

The protocol can work with BNB token because in the documentation is written:

Sablier protocol is compatible with the following:
​
Any network which is EVM compatible
Any ERC20 token

Impact

If the asset is a ERC-20 token that reverts by zero approval, the contracts SablierV2MerkleLT, SablierV2MerkleLT will be not sucessfully deployed and the SablierV2BatchLockup::_approve function will revert. This function is called in _handleTransfer function. That means the revert will disable the functionality of the important functions that rely on _handleTransfer function (createWithTimestampsLT, createWithDurationsLD, createWithDurationsLL, createWithDurationsLT, createWithTimestampsLL, createWithTimestampsLD)

Tools Used

Manual Review

Recommendations

Add a check for the type of asset and use forceApprove only for assets that revert when they are not firstly approved to 0.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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