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

Lack of Withdraw Functionality Leading to Trapped Ether

File Location: protocol/contracts/beanstalk/sun/SeasonFacet/SeasonFacet.sol#L21-L111

Vulnerability Details

The following contracts contain at least one payable function, yet the function does not utilise forwarded ETH, and the contract is missing functionality to withdraw ETH from the contract. This means that funds may become trapped in the contract indefinitely. Consider adding a withdraw/sweep function to contracts that are capable of receiving ether.

Impact

The 'SeasonFacet' contract contains functions that can receive Ether, but it lacks a mechanism to withdraw this Ether. Without a withdraw or sweep function, any Ether sent to the contract would remain trapped indefinitely. This could lead to significant funds being inaccessible, especially if large amounts of Ether are accidentally or intentionally sent to the contract. By adding a withdraw function, the contract owner can safely retrieve any trapped Ether, ensuring that funds are not permanently lost.

Tools Used

  • Inspection manual

  • Solidity

  • Foundry

Recommendations

To fix this problem it is necessary to add a withdraw/sweep function that allows the contract to send the trapped ETH back to the contract owner or to another specified address.

Code Snippet:

L21-L111

contract SeasonFacet is Invariable, Weather {
using LibRedundantMath256 for uint256;
/**
* @notice Emitted when the Season changes.
* @param season The new Season number
*/
event Sunrise(uint256 indexed season);
//////////////////// SUNRISE ////////////////////
/**
* @notice Advances Beanstalk to the next Season, sending reward Beans to the caller's circulating balance.
* @return reward The number of beans minted to the caller.
* @dev No out flow because any externally sent reward beans are freshly minted.
*/
function sunrise() external payable fundsSafu noOutFlow returns (uint256) {
return gm(LibTractor._user(), LibTransfer.To.EXTERNAL);
}
/**
* @notice Advances Beanstalk to the next Season, sending reward Beans to a specified address & balance.
* @param account Indicates to which address reward Beans should be sent
* @param mode Indicates whether the reward beans are sent to internal or circulating balance
* @return reward The number of Beans minted to the caller.
* @dev No out flow because any externally sent reward beans are freshly minted.
*/
function gm(
address account,
LibTransfer.To mode
) public payable fundsSafu noOutFlow returns (uint256) {
require(!s.sys.paused, "Season: Paused.");
require(seasonTime() > s.sys.season.current, "Season: Still current Season.");
uint32 season = stepSeason();
int256 deltaB = stepOracle();
uint256 caseId = calcCaseIdandUpdate(deltaB);
LibGerminate.endTotalGermination(season, LibWhitelistedTokens.getWhitelistedTokens());
LibGauge.stepGauge();
stepSun(deltaB, caseId);
return incentivize(account, mode);
}
/**
* @notice Returns the expected Season number given the current block timestamp.
* {sunrise} can be called when `seasonTime() > s.sys.season.current`.
*/
function seasonTime() public view virtual returns (uint32) {
if (block.timestamp < s.sys.season.start) return 0;
if (s.sys.season.period == 0) return type(uint32).max;
return uint32((block.timestamp - s.sys.season.start) / s.sys.season.period);
}
//////////////////// SEASON INTERNAL ////////////////////
/**
* @dev Moves the Season forward by 1.
*/
function stepSeason() private returns (uint32 season) {
s.sys.season.current += 1;
season = s.sys.season.current;
s.sys.season.sunriseBlock = uint32(block.number); // Note: Will overflow in the year 3650.
emit Sunrise(season);
}
/**
* @param account The address to which the reward beans are sent, may or may not
* be the same as the caller of `sunrise()`
* @param mode Send reward beans to Internal or Circulating balance
* @dev Mints Beans to `account` as a reward for calling {sunrise()}.
*/
function incentivize(address account, LibTransfer.To mode) private returns (uint256) {
uint256 secondsLate = block.timestamp.sub(
s.sys.season.start.add(s.sys.season.period.mul(s.sys.season.current))
);
// reset USD Token prices and TWA reserves in storage for all whitelisted Well LP Tokens.
address[] memory whitelistedWells = LibWhitelistedTokens.getWhitelistedWellLpTokens();
for (uint256 i; i < whitelistedWells.length; i++) {
LibWell.resetUsdTokenPriceForWell(whitelistedWells[i]);
LibWell.resetTwaReservesForWell(whitelistedWells[i]);
}
uint256 incentiveAmount = LibIncentive.determineReward(secondsLate);
LibTransfer.mintToken(C.bean(), incentiveAmount, account, mode);
emit LibIncentive.Incentivization(account, incentiveAmount);
return incentiveAmount;
}
}

Fixed code:

contract SeasonFacet is Invariable, Weather {
using LibRedundantMath256 for uint256;
// Alamat pemilik kontrak
address public owner;
/**
* @notice Emitted when the Season changes.
* @param season The new Season number
*/
event Sunrise(uint256 indexed season);
modifier onlyOwner() {
require(msg.sender == owner, "Only the owner can call this function");
_;
}
constructor() {
owner = msg.sender; // Set pemilik kontrak ke alamat yang mendeploy kontrak
}
//////////////////// SUNRISE ////////////////////
/**
* @notice Advances Beanstalk to the next Season, sending reward Beans to the caller's circulating balance.
* @return reward The number of beans minted to the caller.
* @dev No out flow because any externally sent reward beans are freshly minted.
*/
function sunrise() external payable fundsSafu noOutFlow returns (uint256) {
return gm(LibTractor._user(), LibTransfer.To.EXTERNAL);
}
/**
* @notice Advances Beanstalk to the next Season, sending reward Beans to a specified address & balance.
* @param account Indicates to which address reward Beans should be sent
* @param mode Indicates whether the reward beans are sent to internal or circulating balance
* @return reward The number of Beans minted to the caller.
* @dev No out flow because any externally sent reward beans are freshly minted.
*/
function gm(
address account,
LibTransfer.To mode
) public payable fundsSafu noOutFlow returns (uint256) {
require(!s.sys.paused, "Season: Paused.");
require(seasonTime() > s.sys.season.current, "Season: Still current Season.");
uint32 season = stepSeason();
int256 deltaB = stepOracle();
uint256 caseId = calcCaseIdandUpdate(deltaB);
LibGerminate.endTotalGermination(season, LibWhitelistedTokens.getWhitelistedTokens());
LibGauge.stepGauge();
stepSun(deltaB, caseId);
return incentivize(account, mode);
}
/**
* @notice Returns the expected Season number given the current block timestamp.
* {sunrise} can be called when `seasonTime() > s.sys.season.current`.
*/
function seasonTime() public view virtual returns (uint32) {
if (block.timestamp < s.sys.season.start) return 0;
if (s.sys.season.period == 0) return type(uint32).max;
return uint32((block.timestamp - s.sys.season.start) / s.sys.season.period);
}
//////////////////// SEASON INTERNAL ////////////////////
/**
* @dev Moves the Season forward by 1.
*/
function stepSeason() private returns (uint32 season) {
s.sys.season.current += 1;
season = s.sys.season.current;
s.sys.season.sunriseBlock = uint32(block.number); // Note: Will overflow in the year 3650.
emit Sunrise(season);
}
/**
* @param account The address to which the reward beans are sent, may or may not
* be the same as the caller of `sunrise()`
* @param mode Send reward beans to Internal or Circulating balance
* @dev Mints Beans to `account` as a reward for calling {sunrise()}.
*/
function incentivize(address account, LibTransfer.To mode) private returns (uint256) {
uint256 secondsLate = block.timestamp.sub(
s.sys.season.start.add(s.sys.season.period.mul(s.sys.season.current))
);
// reset USD Token prices and TWA reserves in storage for all whitelisted Well LP Tokens.
address[] memory whitelistedWells = LibWhitelistedTokens.getWhitelistedWellLpTokens();
for (uint256 i; i < whitelistedWells.length; i++) {
LibWell.resetUsdTokenPriceForWell(whitelistedWells[i]);
LibWell.resetTwaReservesForWell(whitelistedWells[i]);
}
uint256 incentiveAmount = LibIncentive.determineReward(secondsLate);
LibTransfer.mintToken(C.bean(), incentiveAmount, account, mode);
emit LibIncentive.Incentivization(account, incentiveAmount);
return incentiveAmount;
}
//////////////////// WITHDRAW FUNCTION ////////////////////
/**
* @notice Withdraws ETH from the contract to the owner's address.
*/
function withdraw() external onlyOwner {
uint256 balance = address(this).balance;
require(balance > 0, "No ETH to withdraw");
(bool success, ) = owner.call{value: balance}("");
require(success, "Withdraw failed");
}
}

Explanation:

  1. Address of the contract owner ('owner') initialized in the constructor.

  2. Modifier 'onlyOwner' to ensure that only the contract owner can call the 'withdraw' function.

  3. The ‘withdraw’ function allows the contract owner to withdraw all ETH in the contract.

Code when testing using Foundry:

contract SeasonFacet {
address public owner;
event Sunrise(uint256 indexed season);
uint256 public season;
modifier onlyOwner() {
require(msg.sender == owner, "Only the owner can call this function");
_;
}
constructor() {
owner = msg.sender;
}
function sunrise() external payable returns (uint256) {
season += 1;
emit Sunrise(season);
return season;
}
function withdraw() external onlyOwner {
uint256 balance = address(this).balance;
require(balance > 0, "No ETH to withdraw");
(bool success, ) = owner.call{value: balance}("");
require(success, "Withdraw failed");
}
receive() external payable {}
}

Foundry output:

Ran 2 tests for test/SeasonFacet.t.sol:SeasonFacetTest
[PASS] testSunrise() (gas: 38615)
[PASS] testWithdraw() (gas: 26203)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 61.25ms (23.24ms CPU time)

Ran 1 test suite in 116.74ms (61.25ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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