Beanstalk Part 1
Beanstalk
DeFiHardhatOracleProxyUpdates
100,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Locked Ether Vulnerability in MigrationFacet Contract

pisces

Summary

Hello security team, hope you are doing well :)
The MigrationFacet contract within the Beanstalk project is designed to facilitate the migration of farmers' deposits from an old silo system to a new one. However, upon reviewing the contract code, it was identified that the contract does not include a function to withdraw Ether, which is a critical security issue. This vulnerability, known as the "locked ether vulnerability," poses a significant risk as it allows Ether to be sent to the contract without providing a mechanism to withdraw it. Consequently, any Ether sent to the contract could be locked indefinitely, leading to potential loss of assets and operational issues.

Vulnerability Details

  1. Deploy the MigrationFacet contract using Hardhat or any Ethereum development environment.

  2. Send Ether to the contract using a transaction.

  3. Attempt to call a non-existent withdrawEther function on the contract.

Proof of Concept (POC) Script:

const { expect } = require("chai");
const hre = require("hardhat");
​
describe("MigrationFacet", function () {
let migrationFacet;
let owner;
​
beforeEach(async function () {
const MigrationFacet = await hre.ethers.getContractFactory("MigrationFacet");
migrationFacet = await MigrationFacet.deploy();
await migrationFacet.deployed();
​
[owner] = await hre.ethers.getSigners();
});
​
it("Should lock Ether sent to the contract", async function () {
const amount = hre.ethers.utils.parseEther("1.0");
​
const tx = await owner.sendTransaction({
to: migrationFacet.address,
value: amount,
});
await tx.wait();
​
const contractBalance = await hre.ethers.provider.getBalance(migrationFacet.address);
expect(contractBalance).to.equal(amount);
​
try {
await migrationFacet.withdrawEther(amount, { from: owner.address });
} catch (error) {
expect(error.message).to.include("revert");
}
});
});

Output:

npx hardhat test test/MigrationFacet.test.js
​
​
MigrationFacet
​
1) Should lock Ether sent to the contract
​
·-----------------------|---------------------------|-------------|-----------------------------·
| Solc version: 0.7.6 · Optimizer enabled: true · Runs: 100 · Block limit: 30000000 gas │
························|···························|·············|······························
| Methods │
·············|··········|·············|·············|·············|···············|··············
| Contract · Method · Min · Max · Avg · # calls · eur (avg) │
·············|··········|·············|·············|·············|···············|··············
| Deployments · · % of limit · │
························|·············|·············|·············|···············|··············
| MigrationFacet · - · - · 2142152 · 7.1 % · - │
·-----------------------|-------------|-------------|-------------|---------------|-------------·
​
0 passing (6s)
1 failing
​
1) MigrationFacet
Should lock Ether sent to the contract:
Error: Transaction reverted: function selector was not recognized and there's no fallback nor receive function

Impact

The locked ether vulnerability poses a significant security risk, as it allows Ether to be sent to the contract without providing a mechanism to withdraw it. This could lead to loss of assets and operational issues, as funds could be trapped in the contract indefinitely.

Tools Used

Manual VS Code review

Recommendations

To address this issue, a withdrawal function should be implemented in the MigrationFacet contract. This function should allow authorized addresses to withdraw Ether from the contract. Here's an example of how such a function might be implemented:

address payable public owner;
​
constructor() {
owner = payable(msg.sender);
}
​
function withdrawEther(uint256 amount) public {
require(msg.sender == owner, "Only the contract owner can withdraw Ether");
require(address(this).balance >= amount, "Insufficient contract balance");
owner.transfer(amount);
}

This fix ensures that Ether sent to the contract can be safely withdrawn by the contract owner, mitigating the locked ether vulnerability and enhancing the security and functionality of the contract.

Updates

Lead Judging Commences

giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational/Invalid

Support

FAQs

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