Beanstalk Part 3

Vulnerability Details

The addUnderlying function within the contract uses the approve method from the ERC20 standard in 2 instances to grant permission to the barnRaiseWell contract to transfer a specific amount of tokens from the contract's address and also to grant the barnRaiseWell contract permission to transfer tokenAmountIn amount of the barnRaiseToken from the contract's address. This is necessary because the barnRaiseWell contract needs to be able to transfer these tokens as part of the liquidity addition process. Without this approval, the barnRaiseWell contract would not be able to move the tokens, and the operation would fail.. However, these implementations does not use the safeApprove method available in OpenZeppelin's SafeERC20 library. The use of plain approve might lead to potential issues due to the allowance manipulation vulnerability, known as approval race condition.

Also, there are many Weird ERC20 Tokens that won't work correctly using the standard IERC20 interface. For example, IERC20(token).transferFrom() and IERC20(token).transfer() will fail for some tokens as they may not conform to the standard IERC20 interface.

Read more here: https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit#heading=h.m9fhqynw2xvt

These are the affected parts of the contract:

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibFertilizer.sol#L152-L154

https://github.com/Cyfrin/2024-05-Beanstalk-3/blob/662d26f12ee219ee92dc485c06e01a4cb5ee8dfb/protocol/contracts/libraries/LibFertilizer.sol#L147

Tools Used

Manual review

Recommendations

To mitigate the potential risks associated with the approve and transferFrom methods, you should consider using the safeApprove and safeTransferfrom method from OpenZeppelin's SafeERC20 library. This will ensure that the contract’s token operations are secure and resistant to known vulnerabilities