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

Use of unsafe Transfer can lead to loss of funds

Summary

PipelineConvertFacet introduces the generalized convert. However, it uses unsafe transfer functions to make transfers from Beanstalk to Pipeline and vice-versa.

The 1st problem with this approach is that Beanstalk does not verify the result of the transfer call, which can fail especially for tokens that do not revert on transfer like USDT.

The 2nd problem is that the Pipeline will not revert when the transfer returns false as it only reverts in two situations: standard revert message or silent revert.

Notice that the PipelineConvertFacet utilizes the fundsSafu which will reduce the impact of the loss while the balance of tokens on beanstalk >= entitlements.

Vulnerability Details

LibConvert -> executePipelineConvert:

function executePipelineConvert(
address inputToken,
address outputToken,
uint256 fromAmount,
uint256 fromBdv,
uint256 initialGrownStalk,
AdvancedFarmCall[] calldata advancedFarmCalls
) external returns (uint256 toAmount, uint256 newGrownStalk, uint256 newBdv) {
...
@> IERC20(inputToken).transfer(C.PIPELINE, fromAmount);
...
}

LibConvert -> transferTokensFromPipeline:

function transferTokensFromPipeline(address tokenOut) internal returns (uint256 amountOut) {
amountOut = IERC20(tokenOut).balanceOf(C.PIPELINE);
require(amountOut > 0, "Convert: No output tokens left in pipeline");
PipeCall memory p;
p.target = address(tokenOut);
@> p.data = abi.encodeWithSelector(IERC20.transfer.selector, address(this), amountOut); // @audit unsafe transfer
IPipeline(C.PIPELINE).pipe(p); // @audit pipeline doesn't revert when transfer returns false
}

Impact

  • User/Beanstalk can lose tokens when it fails from Pipeline -> Beanstalk.

  • Beanstalk can have a broken balance when it fails on Beanstalk -> Pipeline.

Tools Used

Manual Review

Recommendations

Replace transfer with safeTransfer from SafeERC20 from OZ, by doing this the transaction will revert whenever the transfer fails regardless of the token.

Updates

Lead Judging Commences

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

Informational/Gas

Invalid as per docs https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

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