safdie - Lack of input validation for `facetAddress()` function will allow attackers pass arbitrary selectors and flood the contract with useless function calls in `DiamondLoupFacet.sol`

[sherlock-admin4]

safdie

Medium

Lack of input validation for facetAddress() function will allow attackers pass arbitrary selectors and flood the contract with useless function calls in DiamondLoupFacet.sol

Summary

Lack of input validation for facetAddress() function in DiamondLoupFacet.sol will allow attackers pass arbitrary selectors and flood the contract with useless function calls.

Root Cause

The facetAddress() function in the DiamondLoupeFacet contract allows users to pass a bytes4 function selector and retrieve the corresponding facet address. However, this function does not validate the input, potentially leading to several issues. The function lacks validation for whether the input bytes4 _functionSelector is a valid function selector. Attackers can pass arbitrary values to this function, which can degrade contract performance or spam the function with unnecessary calls. https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/DiamondLoup/DiamondLoupFacet.sol#L129-L132

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

An attacker can call the function repeatedly with random or malformed bytes4 values. Since no validation exists, the contract will search the storage for non-existent selectors, potentially resulting in degraded performance. In PoC below attack contract calls facetAddress() 1,000 times, each with a random bytes4 value. This bloats the contract with unnecessary calls, wasting gas and possibly causing the contract to run out of gas in some situations.

Impact

1. Attackers can repeatedly call facetAddress() with random or malformed inputs, forcing the contract to search through the LibDiamond.DiamondStorage structure. This increases gas consumption and may cause legitimate transactions to fail due to higher gas costs or the contract running out of gas during execution.
2. A spam attack of this nature could cause legitimate users to experience significant delays or gas spikes, potentially making certain facets of the diamond contract unusable. Over time, the attacker could cause the contract to hit gas limits, rendering the contract uncallable for legitimate users.
3. In worst-case scenarios, especially with large numbers of selectors and facets, the contract could hit the block gas limit, causing the function to revert due to out-of-gas errors. This would effectively create a DoS attack for legitimate users who need to retrieve the actual facet address for valid function selectors.

PoC

contract FacetAddressSpammer {
    DiamondLoupeFacet diamond;

    constructor(address _diamondAddress) {
        diamond = DiamondLoupeFacet(_diamondAddress);
    }

    function spamFacetAddress() public {
        for (uint256 i = 0; i < 1000; i++) {
            bytes4 randomSelector = bytes4(keccak256(abi.encodePacked(i)));
            diamond.facetAddress(randomSelector);
        }
    }
}

Mitigation

To mitigate this issue, add input validation for the _functionSelector parameter. Ensure that the input is a valid function selector registered within the contract and limit unnecessary calls.

function facetAddress(bytes4 _functionSelector) external view override returns (address facetAddress_) {
    LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage();

    // Validate if the selector exists
    require(ds.facetAddressAndSelectorPosition[_functionSelector].facetAddress != address(0), 
            "DiamondLoupeFacet: Invalid function selector");

    facetAddress_ = ds.facetAddressAndSelectorPosition[_functionSelector].facetAddress;
}

This validation ensures that the function selector must be valid (i.e., associated with a facet) before proceeding, preventing spam with arbitrary selectors.