Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Zero value transfer vulnerability in Silo Deposit functionality

krisp

Summary

A vulnerability exists in Beanstalk's Silo deposit functionality that could cause transactions to revert when depositing certain tokens. Specifically, the issue arises when the transfer amount is zero, which can occur when using the INTERNAL_TOLERANT or EXTERNAL_INTERNAL modes. Some ERC-20 tokens revert on zero value transfers, which would prevent users from depositing their tokens successfully. Currently, Beanstalk does not support such tokens; however, as stated in their documentation Additional tokens may be added to the Silo's Deposit Whitelist, so if this happens, it will result in the aforementioned vulnerability. It could be assumed that Beanstalk would not allow tokens that revert on zero transfers, but this issue could also arise silently if, for example, Beanstalk allows widely used tokens such as USDC or USDT, which are upgradable and whose functionality could change at any time. If the administrators of these tokens decide to add such a restriction for transfers, this will again result in the vulnerability.

Vulnerability Details

When the SiloFacet::deposit() function is called to deposit a whitelisted token into the Silo, it calls LibTransfer::receiveToken(). However, the current implementation of this function has a flaw that will result in a security vulnerability if Beanstalk starts to support tokens that revert when a transfer of zero amount is attempted. Let's analyze the code to see how this bug arises.

LibTransfer::receiveToken()

function receiveToken(
IERC20 token,
uint256 amount,
address sender,
From mode
) internal returns (uint256 receivedAmount) {
if (amount == 0) return 0;
if (mode != From.EXTERNAL) {
receivedAmount = LibBalance.decreaseInternalBalance(
sender,
token,
amount,
mode != From.INTERNAL
);
if (amount == receivedAmount || mode == From.INTERNAL_TOLERANT) return receivedAmount;
}
uint256 beforeBalance = token.balanceOf(address(this));
token.safeTransferFrom(sender, address(this), amount - receivedAmount);
return receivedAmount.add(token.balanceOf(address(this)).sub(beforeBalance));
}

This bug arises if the user wants to transfer their tokens in a mode other than EXTERNAL. In that case, we enter the second if statement of the function, and if the user wants to deposit the entire amount of the token they have internally, this will result in receivedAmount being equal to amount, causing amount - receivedAmount to be zero, which will then be attempted to be transferred here:

token.safeTransferFrom(sender, address(this), amount - receivedAmount);

However, if the token in context is one that reverts on zero transfer amounts, this will lead to a revert, and users will be forced to always leave some amounts in order to avoid this, resulting in a very cumbersome experience for them.

Impact

Users will be unable to deposit their tokens if the transfer amount is zero and the token reverts on zero value transfers. This forces users to always leave a minimal amount of tokens in their balance, which is inconvenient and could lead to negative user experiences.

Tools Used

Manual Code Review

Recommendations

Modify the LibTransfer::receiveToken() function to check if amount - receivedAmount is zero before attempting the transfer. If it is zero, skip the transfer call. This change will ensure compatibility with ERC-20 tokens, that revert on zero value transfers.

function receiveToken(IERC20 token, uint256 amount, address sender, From mode)
internal
returns (uint256 receivedAmount)
{
...
uint256 beforeBalance = token.balanceOf(address(this));
- token.safeTransferFrom(sender, address(this), amount - receivedAmount);
+ uint256 amountToTransfer = amount - receivedAmount;
+ if(amountToTransfer > 0) {
+ token.safeTransferFrom(sender, address(this), amountToTransfer);
}
return receivedAmount.add(token.balanceOf(address(this)).sub(beforeBalance));
}
Updates

Lead Judging Commences

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

Informational/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.