Beanstalk Part 2
Beanstalk
DeFiHardhat
35,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Missing `nonReentrant` modifier

tigerfrake

Summary

The UnripeFacet contract implements the ReentrancyGuard as a shield against reentrancy. All its external/public functions such as addMigratedUnderlying(), addUnripeToken(), pick() & chop() which involve state changes and at the same time interact with other functions from other contracts are protected by nonReentrant modifier.
However, the switchUnderlyingToken() function is missing this modifier when it just as all the other functions operate in a similar manner.

Vulnerability Details

  • https://github.com/Cyfrin/2024-04-beanstalk-2/blob/a3d702c2e108cac6ebdf2416906cbca73c83ec99/protocol/contracts/beanstalk/barn/UnripeFacet.sol#L343-L350

function switchUnderlyingToken(
address unripeToken,
address newUnderlyingToken
) external payable {
LibDiamond.enforceIsContractOwner();
require(s.u[unripeToken].balanceOfUnderlying == 0, "Unripe: Underlying balance > 0");
LibUnripe.switchUnderlyingToken(unripeToken, newUnderlyingToken);
}

This function makes an external call to LibUnripe.switchUnderlyingToken():

function switchUnderlyingToken(address unripeToken, address newUnderlyingToken) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
s.u[unripeToken].underlyingToken = newUnderlyingToken;
emit SwitchUnderlyingToken(unripeToken, newUnderlyingToken);
}

which modifies the underlyingToken attribute of the u mapping entry associated with the unripeToken provided as an argument.
By assigning newUnderlyingToken to s.u[unripeToken].underlyingToken, the function updates the underlying token associated with the unripeToken.

Impact

Since the switchUnderlyingToken() function modifies contract state and involves external calls without proper reentrancy protection, it may be susceptible to reentrancy attacks.

Tools Used

Manual Review

Recommendations

It is in order for this function to have the nonReentrant modifier as the other functions have it as well..

function switchUnderlyingToken(
address unripeToken,
address newUnderlyingToken
) external payable nonReentrant { // @audit-info Protected
LibDiamond.enforceIsContractOwner();
require(s.u[unripeToken].balanceOfUnderlying == 0, "Unripe: Underlying balance > 0");
LibUnripe.switchUnderlyingToken(unripeToken, newUnderlyingToken);
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Re-entrancy modifier

Support

FAQs

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