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

The barn reseed can be reverted by a single user by not accepting the ERC1155 mint

draiakoo

Summary

The barn reseed can be reverted by a single user by not accepting the ERC1155 mint

Relevant GitHub Links:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/init/reseed/L2/ReseedBarn.sol#L88-L93

Vulnerability Details

When Beanstalk will be migrated to an L2 all the state will have to be transfered there. One of the components that need to be reinitialized is the barn. To transfer all the data the following init function will be executed:

function init(
Fertilizers[] calldata fertilizerIds,
uint256 activeFertilizer,
uint256 fertilizedIndex,
uint256 unfertilizedIndex,
uint128 bpf
) external {
// deploy fertilizer implmentation.
Fertilizer fertilizer = new Fertilizer();
​
// deploy fertilizer proxy. Set owner to beanstalk.
TransparentUpgradeableProxy fertilizerProxy = new TransparentUpgradeableProxy(
address(fertilizer),
address(this),
abi.encode(IFertilizer.init.selector)
);
​
mintFertilizers(Fertilizer(address(fertilizerProxy)), fertilizerIds);
s.sys.season.fertilizing = true;
s.sys.fert.activeFertilizer = activeFertilizer;
s.sys.fert.fertilizedIndex = fertilizedIndex;
s.sys.fert.unfertilizedIndex = unfertilizedIndex;
s.sys.fert.bpf = bpf;
}
​
function mintFertilizers(
Fertilizer fertilizerProxy,
Fertilizers[] calldata fertilizerIds
) internal {
for (uint i; i < fertilizerIds.length; i++) {
Fertilizers memory f = fertilizerIds[i];
// set s.firstFid, s.nextFid, s.lastFid
uint128 fid = f.fertilizerId;
if (i == 0) s.sys.fert.fertFirst = fid;
if (i != 0) s.sys.fert.nextFid[fertilizerIds[i - 1].fertilizerId] = fid;
if (i == fertilizerIds.length - 1) s.sys.fert.fertLast = fid;
​
// reissue fertilizer to each holder.
for (uint j; j < f.accountData.length; j++) {
// `id` only needs to be set once per account, but is set on each fertilizer
// as `Fertilizer` does not have a function to set `id` once on a batch.
fertilizerProxy.beanstalkMint(
f.accountData[j].account,
fid,
f.accountData[j].amount,
f.accountData[j].lastBpf
);
}
}
}

As we can see, the it first deploys the fertilizer contract, initializes it and then starts minting the fertilizer to all users that had a position in the L1 contract. To mint a user some fertilizer, the function beanstalkMint is called in the fertilizer contract. Inside there we can see the following:

function beanstalkMint(
address account,
uint256 id,
uint128 amount,
uint128 bpf
) external onlyOwner {
if (_balances[id][account].amount > 0) {
uint256[] memory ids = new uint256[](1);
ids[0] = id;
_update(account, ids, bpf);
}
_balances[id][account].lastBpf = bpf;
_safeMint(account, id, amount, bytes("0"));
}

At the end of the execution, the _safeMint function is called. This function is an ERC1155 specific function that checks if the receiver can handle ERC1155 tokens:

function _safeMint(address to, uint256 id, uint256 amount, bytes memory data) internal virtual {
require(to != address(0), "ERC1155: mint to the zero address");
​
address operator = _msgSender();
​
_transfer(address(0), to, id, amount);
​
emit TransferSingle(operator, address(0), to, id, amount);
​
__doSafeTransferAcceptanceCheck(operator, address(0), to, id, amount, data);
}
​
​
function __doSafeTransferAcceptanceCheck(
address operator,
address from,
address to,
uint256 id,
uint256 amount,
bytes memory data
) private {
if (LibFertilizer.isContract(to)) {
try IERC1155Receiver(to).onERC1155Received(operator, from, id, amount, data) returns (
bytes4 response
) {
if (response != IERC1155Receiver.onERC1155Received.selector) {
revert("ERC1155: ERC1155Receiver rejected tokens");
}
} catch Error(string memory reason) {
revert(reason);
} catch {
revert("ERC1155: transfer to non ERC1155Receiver implementer");
}
}
}

This can be a problem because when the protocol will try to mint the fertilizer to all the users passed in the init function, if any of them is a contract and does not have the onERC1155Received callback function implemented or intentionally returns a different response, the whole init function to initialize the barn in the L2 will revert.

Impact

High, any of the fertilizer recipients can easily prevent the execution of the initialization of the barn for free

Tools Used

Manual review

Recommendations

I would not recommend migrating positions by the protocol because since the ERC1155 has to be accepted by the receiver he can make it revert. I would change make the user trigger a migrate function on L1 that would send a message on L2 that would only mint the fertilizer to the specific user. So if the user does not accept the ERC1155 it will only revert his own mint and will not harm others.

Updates

Lead Judging Commences

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

`_safeMint` can DOSS due to a receiver not implementing `onERC1155Received`

Support

FAQs

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