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

L2ContractMigrationFacet for Silo signature can be replayed

asefewwexa

Line of code

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/silo/L2ContractMigrationFacet.sol#L88

Summary

L2Migration for Silo signature can be replayed

Vulnerability Details

function redeemDepositsAndInternalBalances(
address owner,
address reciever,
AccountDepositData[] calldata deposits,
AccountInternalBalance[] calldata internalBalances,
uint256 ownerRoots,
bytes32[] calldata proof,
uint256 deadline,
bytes calldata signature
) external payable fundsSafu noSupplyChange nonReentrant {
// verify deposits are valid.
// note: if the number of contracts that own deposits is small,
// deposits can be stored in bytecode rather than relying on a merkle tree.
verifyDepositsAndInternalBalances(owner, deposits, internalBalances, ownerRoots, proof);
​
// signature verification.
verifySignature(owner, reciever, deadline, signature);
​
// set deposits for `reciever`.
uint256 accountStalk;
for (uint256 i; i < deposits.length; i++) {
accountStalk += addMigratedDepositsToAccount(reciever, deposits[i]);
}
​
// set stalk for account.
setStalk(reciever, accountStalk, ownerRoots);
}

When depositing in the migration faucet, the deposit info must be in the merkle root and user signature must be valid.

function verifyDepositsAndInternalBalances(
address account,
AccountDepositData[] calldata deposits,
AccountInternalBalance[] calldata internalBalances,
uint256 ownerRoots,
bytes32[] calldata proof
) internal pure {
bytes32 leaf = keccak256(abi.encode(account, deposits, internalBalances, ownerRoots));
require(MerkleProof.verify(proof, MERKLE_ROOT, leaf), "Migration: invalid proof");
}

The function above will verify that the info is included in the merkle root. The problem is that the signature and proof is never checked if proof and signature is already used.

Becase of this the signature and merkle proof can be used more than once and replayed for deposits in the migration contract

Impact

Becase of this the signature and merkle proof can be used more than once and replayed for deposits in the migration contract to get the balance multiple times.

Tools Used

manual review

Recommendations

the code should validate that the signature cannot be replayed and the user should be only add the balance once (check if the data is in merkle root and if the user balance is alread added.)

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Replay attack vulnerability in `redeemDepositsAndInternalBalances` function - it could be replayed

Support

FAQs

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