fallback allows invalid facet addresses to be used
Summary
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
Root Cause
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.
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
No response
Impact
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
PoC
No response
Mitigation
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.
air_0x
Medium
fallback allows invalid facet addresses to be used
Summary
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
Root Cause
The fallback function doesn't verify if there's actually code at the
facetaddress before making thedelegatecall.After the
delegatecall, there's also no verification of the returned data length as seen in the code below :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.
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
No response
Impact
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
delegatecalcall to non existence facet address will still executePoC
No response
Mitigation
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.