Beanstalk: The Finale

Summary

This report identifies a potential issue in SiloFacet.sol that could lead to locked ETH within the contract. The issue is related to several functions being marked as payable even though they do not process ETH payments directly.

Vulnerability Details

Several functions within the SiloFacet contract are marked as payable even though they don't handle ETH payments. These functions are:

deposit
withdrawDeposit
withdrawDeposits
transferDeposit
transferDeposits
safeTransferFrom
safeBatchTransferFrom

If any of these functions are accidentally or intentionally called with a value greater than 0 ETH, the sent ETH will be locked in the contract and inaccessible.

Impact

Loss of funds: Any ETH accidentally or intentionally sent through the listed functions will be locked in the contract and inaccessible.

Tools Used

Manual code review

Recommendations

Remove the payable modifier from all the functions listed above. These functions currently do not handle ETH payments and making them non-payable will prevent accidental locking of ETH.

If future functionalities within the SiloFacet require ETH payments for specific purposes related to deposits, withdrawals, or transfers, implement a secure mechanism to handle them. This mechanism should clearly explain the purpose of ETH payments and ensure proper processing and withdrawal for authorized users.