Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

DepotFacet disregards lack of fallback function for callee when initiating refunds

golanger85

Summary

The DepotFacet contract interacts with the Pipeline contract and attempts to refund any remaining Ether to the caller at the end of the function execution using the LibEth.refundEth() function.

However, if the callee does not have a payable fallback function, the refund will fail, causing the entire transaction to revert. This can lead to unexpected gas losses for users.

/**
* @notice Pipe multiple AdvancedPipeCalls through Pipeline.
* @param pipes list of AdvancedPipeCalls to pipe through Pipeline
* @return results list of return values from each AdvancedPipeCall
**/
function advancedPipe(AdvancedPipeCall[] calldata pipes,uint256 value)
external payable fundsSafu noSupplyIncrease returns (bytes[] memory results) {
results = IPipeline(PIPELINE).advancedPipe{value: value}(pipes);
LibEth.refundEth();
}
​
/**
* @notice Pipe a PipeCall through Pipeline with an Ether value.
* @param p PipeCall to pipe through Pipeline
* @param value Ether value to send in Pipecall
* @return result PipeCall return value
**/
function etherPipe(PipeCall calldata p,uint256 value)
external payable fundsSafu noSupplyIncrease returns (bytes memory result) {
result = IPipeline(PIPELINE).pipe{value: value}(p);
LibEth.refundEth();
}
​

Vulnerability Details

The vulnerability arises from the following code in the LibEth.refundEth() function:

if (address(this).balance > 0 && s.isFarm != 2) {(bool success, ) = msg.sender.call{value: address(this).balance}(new bytes(0));require(success, "Eth transfer Failed.");}

Impact

Any transaction that involves a call to the LibEth.refundEth() function will revert if the caller’s contract does not have a payable fallback function. This means all state changes made before the refund attempt will be rolled back, which in turn, users will lose the gas fees spent on the transaction due to the reversion caused by the failed Ether transfer. They will have to call this function again and lose more funds on gas.


Consider the following scenario:

Alice creates a non-payable contract that interacts with the DepotFacet contract.
Alice’s contract makes a call to the advancedPipe function in the DepotFacet contract.
The LibEth.refundEth() function attempts to refund remaining Ether to Alice’s contract.
Since Alice’s contract does not have a payable fallback function, the refund fails, causing the entire transaction to revert and Alice loses the gas fees spent on the transaction.

Tools Used

Manual Review

Recommendations

Introduce a Secondary Refund Address with Conditional Statements.


Modify the LibEth.refundEth() function to include a secondary refund address as an optional parameter. This allows specifying an alternative address for refunds if the original caller cannot accept Ether.

Also, update DepotFacet functions to handle optional secondary refund address.

Pass an optional secondary address to the LibEth.refundEth function that can be used if the primary refund to msg.sender fails.

By implementing these changes, the contract ensures that if the primary caller cannot receive Ether, the refund will be sent to an alternative address specified within the function parameters that is able to receive the remaining balance afterwards.

Updated DepotFacet functions:

function advancedPipe(AdvancedPipeCall[] calldata pipes, uint256 value, address refundAddress)
external payable fundsSafu noSupplyIncrease returns (bytes[] memory results) {
results = IPipeline(PIPELINE).advancedPipe{value: value}(pipes);
LibEth.refundEth(refundAddress);
}
​
function etherPipe(PipeCall calldata p, uint256 value, address refundAddress)
external payable fundsSafu noSupplyIncrease returns (bytes memory result) {
result = IPipeline(PIPELINE).pipe{value: value}(p);
LibEth.refundEth(refundAddress);
}

Updated LibEth.refundEth() function:

library LibEth {
function refundEth(address refundAddress) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
if (address(this).balance > 0 && s.isFarm != 2) {
address payable recipient = payable(msg.sender);
(bool success, ) = recipient.call{value: address(this).balance}(new bytes(0));
if (!success && refundAddress != address(0)) {
recipient = payable(refundAddress);
(success, ) = recipient.call{value: address(this).balance}(new bytes(0));
}
require(success, "Eth transfer Failed.");
}
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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