Beanstalk: The Finale

Summary

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/9c7b9fd521ad7cbe65cc788df181887c0eb39c6d/protocol/contracts/beanstalk/Diamond.sol#L33-L53

The contract is Standard allows for modular contract development using the "facet" pattern, which can lead to complex interactions between different parts of the contract and it's contain a reentrancy vulnerability from the missing of the nonReentrant modifier.

Vulnerability Details

The contract use fallback function and employs delegatecall to forward external calls to addresses determined by the selectorToFacetAndPosition mapping and this setup presents a risk of reentrancy attacks, because the contract itself does not have a reentrancy guard. This means each facet, especially those with state-changing and payable functions, must independently implement reentrancy protection measures. If these facets lack appropriate guards, they can be vulnerable to reentrancy attacks. In such attacks, an adversary could exploit the absence of reentrancy protection to perform unexpected state changes or asset transfers by recursively calling the function. here is the vulnerable part :

`fallback() external payable {
    LibDiamond.DiamondStorage storage ds;
    bytes32 position = LibDiamond.DIAMOND_STORAGE_POSITION;
    assembly {
        ds.slot := position
    }
    address facet = ds.selectorToFacetAndPosition[msg.sig].facetAddress;
    require(facet != address(0), "Diamond: Function does not exist");
    assembly {
        calldatacopy(0, 0, calldatasize())
        let result := delegatecall(gas(), facet, 0, calldatasize(), 0, 0)
        returndatacopy(0, 0, returndatasize())
        switch result
        case 0 {
            revert(0, returndatasize())
        }
        default {
            return(0, returndatasize())
        }
    }
}`

An attacker can potentially leverage this vulnerability to manipulate contract states or extract funds,

Impact

Without reentrancy guards, an attacker could potentially exploit this vulnerability and manipulate the contract's state and then can drain funds. This is especially risk for state-changing and payable functions within the facets.

Tools Used

Manual Review

Recommendations

Please add the nonReentrant modifier on the fallback function.