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

[M] Unhandled return value in addUnderlying leading to incorrect contract state

golanger85

Summary

The addUnderlying function in the Beanstalk protocol uses the transferFrom method to move barnRaiseToken from
the user's address to the contract. However, the function does not validate the success of this operation, which can lead to unhandled exceptions if the token transfer fails. This could result in partial execution of the function, where some state changes are committed while others are not, potentially leading to inconsistent state.

Vulnerability Details

The ```transferFrom`` function is used to transfer tokens on behalf of users, which requires prior approval.

The ERC-20 standard does not mandate transferFrom to return a boolean indicating success, and failing silently is common in many token
implementations. In the context of Beanstalk's addUnderlying function, failure to handle these potential exceptions can disrupt the transaction flow, leading to failure in minting or adding liquidity as intended.

C.bean().mint(address(this), newDepositedLPBeans);
​
IERC20(barnRaiseToken).transferFrom(msg.sender, address(this), uint256(tokenAmountIn));
​
IERC20(barnRaiseToken).approve(barnRaiseWell, uint256(tokenAmountIn));
C.bean().approve(barnRaiseWell, newDepositedLPBeans);
​
uint256[] memory tokenAmountsIn = new uint256[](2);
IERC20[] memory tokens = IWell(barnRaiseWell).tokens();
(tokenAmountsIn[0], tokenAmountsIn[1]) = tokens[0] == C.bean() ?
(newDepositedLPBeans, tokenAmountIn) :
(tokenAmountIn, newDepositedLPBeans);
​
uint256 newLP = IWell(barnRaiseWell).addLiquidity(
tokenAmountsIn,
minAmountOut,
address(this),
type(uint256).max//@audit
);
​
// Increment underlying balances of Unripe Tokens
LibUnripe.incrementUnderlying(C.UNRIPE_BEAN, newDepositedBeans);
LibUnripe.incrementUnderlying(C.UNRIPE_LP, newLP);
​
s.recapitalized = s.recapitalized.add(usdAmount);
}

Impact

If transferFrom doesn't revert upon failure, the addUnderlying function will continue to execute, potentially resulting in an approval being granted without the corresponding transfer.

This will also lead to a cascading effect on the rest of addUnderlying's logic.

Tools Used

Manual Review

Recommendations

To mitigate this issue, Beanstalk should implement a check to ensure that the transferFrom operation is successful.

Use Openzeppelin's safeTransferFrom library.

Updates

Lead Judging Commences

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

Unchecked transfers

Support

FAQs

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