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

Asset duplication and price inflation risk due to lack of asset burn mechanism in L2 contract migration

whitehat777

Summary

The L2ContractMigrationFacet contract facilitates migration of assets from L1 to L2 but lacks mechanisms to burn or reduce assets on L1 post-migration. This oversight could lead to unintended asset duplication and potential inflation of asset prices on L2 due to multiple transfers. Additionally, due to the fact that the contract does not enforce asset reduction after migration, which could allow assets to remain active and transferable on L1.

Vulnerability Details

Asset Transfer Flow:

The L2ContractMigrationFacet contract facilitates asset migration from L1 to L2. The redeemDepositsAndInternalBalances function verifies deposits and internal balances using Merkle proofs (verifyDepositsAndInternalBalances), then proceeds to transfer these assets to the recipient on L2 (addMigratedDepositsToAccount). However, if we examine how the addMigratedDepositsToAccount function is implemented, we will notice that it only increases deposit balances of the receiver on L2, and there is no mechanism within it (or within the contract as a whole) to invalidate, reduce, or at least let the contract from which the migration is happening on L1 to know that it should burn these assets on L1 after they are migrated to L2. This potentially allows assets to remain active and transferable on both chains.

L2ContractMigrationFacet#L152-L200

function addMigratedDepositsToAccount(address account, AccountDepositData calldata depositData)
internal
returns (uint256 accountStalk)
{
int96 stemTip = LibTokenSilo.stemTipForToken(depositData.token);
uint256 stalkIssuedPerBdv = s.sys.silo.assetSettings[depositData.token].stalkIssuedPerBdv;
uint128 totalBdvForAccount;
uint128 totalDeposited;
uint128 totalDepositedBdv;
for (uint256 i; i < depositData.depositIds.length; i++) {
// verify that depositId is valid.
uint256 depositId = depositData.depositIds[i];
(address depositToken, int96 stem) = depositId.unpackAddressAndStem();
require(depositToken == depositData.token, "Migration: INVALID_DEPOSIT_ID");
require(stemTip >= stem, "Migration: INVALID_STEM");
​
// add deposit to account.
s.accts[account].deposits[depositId].amount = depositData.amounts[i];
s.accts[account].deposits[depositId].bdv = depositData.bdvs[i];
​
// increment totalBdvForAccount by bdv of deposit:
totalBdvForAccount += depositData.bdvs[i];
​
// increment by grown stalk of deposit.
accountStalk += uint96(stemTip - stem) * depositData.bdvs[i];
​
// emit events.
emit AddDeposit(account, depositData.token, stem, depositData.amounts[i], depositData.bdvs[i]);
emit TransferSingle(msg.sender, address(0), account, depositId, depositData.amounts[i]);
}
​
// update mowStatuses for account and token.
s.accts[account].mowStatuses[depositData.token].bdv = totalBdvForAccount;
s.accts[account].mowStatuses[depositData.token].lastStem = stemTip;
​
// set global state
s.sys.silo.balances[depositData.token].deposited = totalDeposited;
s.sys.silo.balances[depositData.token].depositedBdv = totalDepositedBdv;
​
// increment stalkForAccount by the stalk issued per BDV.
// placed outside of loop for gas effiency.
accountStalk += stalkIssuedPerBdv * totalBdvForAccount;
}

Exploit Scenario

An attacker could exploit the absence of asset burning on L1 by repeatedly calling redeemDepositsAndInternalBalances with a valid signature before the deadline. This could result in multiple migrations of the same assets to L2, causing asset duplication and potentially inflating asset prices on L2 due to increased availability.

Impact

  1. Asset Duplication: Assets may remain active and transferable on L1 after migration to L2, leading to unintended duplication across chains. This would allow malicious users to profit unfairly by transferring the assets multiple times between L1 and L2, potentially increasing their wealth. Additionally, they could sell their L1 assets for further profit.

  2. Price Inflation: The increased availability of duplicated assets on L2 could inflate their prices and disrupt market dynamics, resulting in financial losses for non-malicious users holding these assets.

Tools Used

Manual code review

Recommendations

Implement a mechanism within L2ContractMigrationFacet to burn or reduce assets on L1 after successful migration to L2.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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