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

Missing check of return value of transfer and transferFrom

kind0devpro

Summary

In the "addUnderlying" function of the "LibFertilizer.sol" contract stems from the lack of proper return value checks when interacting with ERC20 tokens. Specifically, the function fails to validate the return values of the "transferFrom" and approve calls, which can lead to severe security risks.

Vulnerability Details

The function neglects to verify the return values of the "transferFrom" and "approve" calls made to interact with ERC20 tokens "(barnRaiseToken)". According to the ERC20 standard, these functions should return a boolean value indicating the success or failure of the operation. However, certain tokens like BAT may deviate from this standard, potentially returning false or failing silently instead of reverting. Since the "addUnderlying" function handles the minting of tokens and liquidity addition to the Barn Raise Well, any successful exploitation of this vulnerability could undermine the system's integrity and functionality.

function addUnderlying(uint256 tokenAmountIn, uint256 usdAmount, uint256 minAmountOut) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
// Calculate how many new Deposited Beans will be minted
uint256 percentToFill = usdAmount.mul(C.precision()).div(
remainingRecapitalization()
);
​
uint256 newDepositedBeans;
if (C.unripeBean().totalSupply() > s.u[C.UNRIPE_BEAN].balanceOfUnderlying) {
newDepositedBeans = (C.unripeBean().totalSupply()).sub(
s.u[C.UNRIPE_BEAN].balanceOfUnderlying
);
newDepositedBeans = newDepositedBeans.mul(percentToFill).div(
C.precision()
);
}
​
// Calculate how many Beans to add as LP
uint256 newDepositedLPBeans = usdAmount.mul(C.exploitAddLPRatio()).div(
DECIMALS
);
​
// Mint the Deposited Beans to Beanstalk.
C.bean().mint(
address(this),
newDepositedBeans
);
​
// Mint the LP Beans and add liquidity to the well.
address barnRaiseWell = LibBarnRaise.getBarnRaiseWell();
address barnRaiseToken = LibBarnRaise.getBarnRaiseToken();
​
C.bean().mint(
address(this),
newDepositedLPBeans
);
​
IERC20(barnRaiseToken).transferFrom(
msg.sender,
address(this),
uint256(tokenAmountIn)
);
....;
​
​

Impact

The absence of proper return value checks poses a risk wherein transfer or approval operations may fail without the contract's awareness. Consequently, tokens could become lost or trapped within the contract, lacking mechanisms to address failed token transfers. This, in turn, may lead to unintended minting of free bean tokens, LP beans, and liquidity addition to the well.

Tools Used

The vulnerability was identified using Slither-analyzer along with manual code review.

Recommendations

It is advised to address the aforementioned instance by implementing proper return value checks. Consider leveraging "safeTransferFrom" from OpenZeppelin, as it is utilized elsewhere in the codebase.

function addUnderlying(uint256 tokenAmountIn, uint256 usdAmount, uint256 minAmountOut) internal {
__;
- ERC20(barnRaiseToken).transferFrom(
+ ERC20(barnRaiseToken).safeTransferFrom(
msg.sender,
address(this),
uint256(tokenAmountIn)
);
....;
}
Updates

Lead Judging Commences

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

Unchecked transfers

Support

FAQs

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