Decent Sepia Caterpillar
Medium
The fallback function in the Diamond contract executes delegatecall to facet addresses without verifying if code exists at those addresses. This allows calls to empty facet addresses to succeed
The fallback function doesn't verify if there's actually code at the facet
address before making the delegatecall
.
After the delegatecall
, there's also no verification of the returned data length as seen in the code below :
fallback() external payable {
LibDiamond.DiamondStorage storage ds;
bytes32 position = LibDiamond.DIAMOND_STORAGE_POSITION;
// get diamond storage
assembly {
ds.slot := position
}
// get facet from function selector
address facet = ds.facetAddressAndSelectorPosition[msg.sig].facetAddress;
require(facet != address(0), "Diamond: Function does not exist");
// Execute external function from facet using delegatecall and return any value.
assembly {
// copy function selector and any arguments
calldatacopy(0, 0, calldatasize())
//@audit-issue : the size of the facet address not checked
//@audit-issue : external call made to facet not checked
let result := delegatecall(gas(), facet, 0, calldatasize(), 0, 0)
// get any return value
returndatacopy(0, 0, returndatasize())
// return any return value or error back to the caller
switch result
case 0 {
revert(0, returndatasize())
}
default {
return(0, returndatasize())
}
}
}
Since calls to addresses without code always succeed, this could lead to bypassing critical logic if an attacker manages to manipulate the diamond storage to point to an empty address.
No response
No response
No response
A call to an address without deployed code always succeeds . This means that if an attacker manages to manipulate the diamond storage to point a function selector to an empty address, the delegatecal
call to non existence facet address will still execute
No response
1 .Before making the external call, the size of the contract should be checked to ensure it has code. 2 . If the external call is successful, the length of the returned data should be verified to match the expected length.