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

Same signature can be reused in ``redeemDepositsAndInternalBalances()`` function of L2ContractMigrationFacet.sol as nonce is not used.

Summary

Same signature can be reused in redeemDepositsAndInternalBalances() function of L2ContractMigrationFacet.sol as nonce is not used.

Vulnerability Details

redeemDepositsAndInternalBalances() function is used to redeem deposits and bean-asset internal balances on L2. For this it uses merkle proof and signature.

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);
}

The problem is this function doesn't check if the assets are already redeemed and signature doesn't have nonce value. Thus, same signature and proof can be used again and again to redeem multiple times on L2.

function verifySignature(address owner, address reciever, uint256 deadline, bytes calldata signature)
internal
view
{
require(block.timestamp <= deadline, "Migration: permit expired deadline");
bytes32 structHash = keccak256(abi.encode(REDEEM_DEPOSIT_TYPE_HASH, owner, reciever, deadline));
bytes32 hash = _hashTypedDataV4(structHash);
address signer = ECDSA.recover(hash, signature);
require(signer == owner, "Migration: permit invalid signature");
}

Impact

redeemDepositsAndInternalBalances() function can be called again and again to increase the internal balances and stalk using the same signature and proof. Thus, protocol can be drained.

Tools Used

Manual Analysis

Relevant Links

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

Recommendations

Add nonce in the calculation of structHash in verifySignature() function and increment it every time the signature is used.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

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

Appeal created

deadrosesxyz Auditor
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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