XDZIBEC - Reentrancy Vulnerability in EIP-2535 Diamond Standard Implementation

[sherlock-admin]

XDZIBEC

high

Reentrancy Vulnerability in EIP-2535 Diamond Standard Implementation

Summary

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 Detail

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.

Code Snippet

• https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/Diamond.sol#L46C1-L67C1

Tool used

Manual Review

Recommendation

need to add the nonReentrant modifier on the fallback function.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Not seeing any re-entrancy attack vectors from current codebase. Even though there is, it will be block at implementation, not entirely on fallback

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Not seeing any re-entrancy attack vectors from current codebase. Even though there is, it will be block at implementation, not entirely on fallback

[molecula451]

Re-opening to check things on the standard-2535

[nevillehuang]

Invalid, insufficient proof that reentrancy can occur.