DiamondLib Manual Review Findings

DLI-01M: Inexistent Requirement of Code

Type	Severity	Location
Input Sanitization		DiamondLib.sol:L227-L233

Description:

The DiamondLib::addFacet function does not mandate that the target _facetAddress has any code within it, permitting targets with no code to be defined.

Impact:

All delegatecall instructions to an address with no code will "succeed" silently which would be incorrect behaviour on behalf of the Diamond implementation.

Example:

Copy
224/// @notice Adds a facet to the diamond
225/// @param ds The diamond storage pointer
226/// @param _facetAddress The facet address to add
227function addFacet(DiamondStorage storage ds, address _facetAddress) internal {
228  ds.facetFunctionSelectors[_facetAddress].facetAddressPosition = ds
229    .facetAddresses
230    .length;
231
232  ds.facetAddresses.push(_facetAddress);
233}

Recommendation:

We advise the DiamondLib::addFacet function to mandate that the _facetAddress has code within it akin to the original EIP-2535 Diamond standard implementation version 3, preventing delegatecall instructions from being performed to targets with no code.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

The code of DiamondLib::addFacet properly validates that the target is a contract via the CallLib::isContract implementation, alleviating this exhibit in full.