DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: medium
Invalid

The implementation logic of `Invariable::fundsSafu` modifier can lead to Denial-Of-Service

Summary

Beanstalk implements "invariant" modifiers that makes sure the invariants of the protocol hold "true" in all cases. Since the protocol is a complex one, this adds additional layer of security during function executions. However if we look at the fundsSafu modifier (which checks how much "debt" it owes to users and if it has enough to return in case of withdrawals), in short it retrieves the tokens which accrue interest over time and checks their balances.

modifier fundsSafu() {
_;
address[] memory tokens = getTokensOfInterest();
(
uint256[] memory entitlements,
uint256[] memory balances
) = getTokenEntitlementsAndBalances(tokens);
...
}
function getTokensOfInterest() internal view returns (address[] memory tokens) {
address[] memory whitelistedTokens = LibWhitelistedTokens.getWhitelistedTokens();
address[] memory sopTokens = LibWhitelistedTokens.getSopTokens();
uint256 totalLength = whitelistedTokens.length + sopTokens.length;
tokens = new address[](totalLength);
for (uint256 i = 0; i < whitelistedTokens.length; i++) {
tokens[i] = whitelistedTokens[i];
}
for (uint256 i = 0; i < sopTokens.length; i++) {
tokens[whitelistedTokens.length + i] = sopTokens[i];
}
}
function getWhitelistedTokens() internal view returns (address[] memory tokens) {
...
for (uint256 i = 0; i < numberOfSiloTokens; i++) {
if (s.sys.silo.whitelistStatuses[i].isWhitelisted) {
tokens[tokensLength++] = s.sys.silo.whitelistStatuses[i].token;
}
}
...
}
function getSopTokens() internal view returns (address[] memory) {
address[] memory tokens = getSoppableWellLpTokens(); // @audit this one also contains loop
for (uint256 i = 0; i < tokens.length; i++) {
tokens[i] = address(LibWell.getNonBeanTokenFromWell(tokens[i]));
}
...
}
function getTokenEntitlementsAndBalances(
address[] memory tokens
) internal view returns (uint256[] memory entitlements, uint256[] memory balances) {
...
for (uint256 i; i < tokens.length; i++) { // @audit one more loop
...

Vulnerability Details

As can be observed in the code snippets above, the modifier makes calls to functions, which contain multiple loops, especially the Invariable::getTokenEntitlementsAndBalances is large and complex one. This drastically increases the gas costs for the modifier's successful execution, which can make it exceed the block gas limit. Moreover the whitelisted tokens can grow in size, and also first the function logic is executed (where the fundsSafu modifier is put in) and then the modifier logic, all this increases the chances reaching the block gas limit, which is currently 30m on Mainnet.

Impact

The functions which include the fundsSafu modifier (which is basically every public or external function) can simply revert every time.

Impact: High, DoS or high gas costs for users
Likelihood: Low, as it requires large arrays length
Overall: Medium

Tools Used

Manual Review

Recommendations

Limit the size of the arrays to reasonable amount, which looping through will not exceed the 30m threshold.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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