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

Delegatecall in FarmFacet lacks checks for facet validity, causing potential loss of funds for users

golanger85

Summary

The use of low level delegatecall in the FarmFacet and LibFarm contracts poses a significant risk if the facetcontract is self-destructed or if there's no code at the target address. This issue is not sufficiently handled by the current implementation, which also doesn't consider the msg.value sent along with the call,
considering the functions are payable.

Vulnerability Details

The delegatecall opcode in Solidity returns success even if there is no contract code at the target address.
This situation can generally occur if the contract being delegate-called has been self-destructed or if the address is
not a contract. The current implementation of LibFarm does not check for the existence of contract code at
the facet address before performing the delegatecall, which can lead to potential loss of funds.

function farm(bytes[] calldata data) external payable fundsSafu withEth returns (bytes[] memory results) {
results = new bytes[](data.length);
for (uint256 i; i < data.length; ++i) {
results[i] = LibFarm._farm(data[i]);
}
// delegatecall a Beanstalk function using memory data
function _farm(bytes memory data) internal returns (bytes memory result) {
bytes4 selector;
bool success;
assembly {
selector := mload(add(data, 32))
}
address facet = LibFunction.facetForSelector(selector);
(success, result) = facet.delegatecall(data);
LibFunction.checkReturn(success, result);
}

Impact

Contracts using FarmFacet and LibFarm rely on delegatecall to execute functions defined in various facets.
If the facet address is self-destructed, The delegatecall will return success, potentially causing loss of funds for the user.

Function calls will silently fail because of the behaviour of the delegatecall opcode.

Tools Used

Manual Review

Recommendations

To mitigate this issue, the LibFarm library should be updated to include a check for the existence of
contract code at the facet address before performing the delegatecall. Here is an updated implementation
of the _farm function that includes this check:

// delegatecall a Beanstalk function using memory data
function _farm(bytes memory data) internal returns (bytes memory result) {
bytes4 selector;
bool success;
assembly {
selector := mload(add(data, 32))
}
address facet = LibFunction.facetForSelector(selector);
​
// Explicitly check that the facet address contains code
require(isContract(facet), "LibFarm: facet has no code");
​
(success, result) = facet.delegatecall(data);
LibFunction.checkReturn(success, result);
}
​
// Function to check if an address is a contract
function isContract(address account) internal view returns (bool) {
uint256 size;
assembly {
size := extcodesize(account)
}
return size > 0;
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 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.