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

`BeanL1RecieverFacet#recieveL1Beans()` would never work

bauchibred

Summary

BeanL1RecieverFacet#recieveL1Beans()will never work due to the hardcoded value of EXTERNAL_L1_BEANS as 0 which causes the attempt to claim these after the burning on L1 of the tokens to always get reverted here.

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/migration/BeanL1RecieverFacet.sol#L31-L44

function recieveL1Beans(address reciever, uint256 amount) external nonReentrant {
// verify msg.sender is the cross-chain messenger address, and
// the xDomainMessageSender is the L1 Beanstalk contract.
require(
msg.sender == address(BRIDGE) &&
IL2Messenger(BRIDGE).xDomainMessageSender() == L1BEANSTALK
);
s.sys.migration.migratedL1Beans += amount;
require(
EXTERNAL_L1_BEANS >= s.sys.migration.migratedL1Beans,
"L2Migration: exceeds maximum migrated"
);
C.bean().mint(reciever, amount);
}

This function is used when attempting to migrate any amount of Beans to L2, i.e it's called in the instance below where the amount of beans are minted to the receiver, https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/migration/BeanL2MigrationFacet.sol#L43-L57

function migrateL2Beans(
address reciever,
address L2Beanstalk,
uint256 amount,
uint32 gasLimit
) external nonReentrant {
C.bean().burnFrom(msg.sender, amount);
​
// send data to
IL2Bridge(BRIDGE).sendMessage(
L2Beanstalk,
abi.encodeCall(IBeanL1RecieverFacet(L2Beanstalk).recieveL1Beans, (reciever, amount)),
gasLimit
);
}

Issue however is the check require(EXTERNAL_L1_BEANS >= s.sys.migration.migratedL1Beans) would always revert in recieveL1Beans() because EXTERNAL_L1_BEANS has been erroneously hardcoded to 0, see https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/migration/BeanL1RecieverFacet.sol#L22

uint256 constant EXTERNAL_L1_BEANS = 0;

Which would then mean that for any valid amount being passed to migrate the migration always fails with the "L2Migration: exceeds maximum migrated" error since the maximum migrated is being set as 0.

Impact

High, considering when migrating the tokens gets burnt from the user and then the message is sent to the L2Bridge with the amount of assets to give to the user, but since the attempt to claim these tokens always gets reverted here, users have lost their Beans.

Tools Used

Manual review

Recommendations

Consider attaching a valid value to EXTERNAL_L1_BEANS, if users can still get more external tokens after the initial migration then this value should be updatable and a logic to update this can be attached to the message that gets sent when migration of Beans is to occur.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

`EXTERNAL_L1_BEANS` defined with `0` will fail require(EXTERNAL_L1_BEANS >= s.sys.migration.migratedL1Beans, "L2Migration: exceeds maximum migrated");

Appeal created

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

`EXTERNAL_L1_BEANS` defined with `0` will fail require(EXTERNAL_L1_BEANS >= s.sys.migration.migratedL1Beans, "L2Migration: exceeds maximum migrated");

Support

FAQs

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