Skip to content

Latest commit

 

History

History
77 lines (53 loc) · 2.61 KB

File metadata and controls

77 lines (53 loc) · 2.61 KB

Decent Sepia Caterpillar

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 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.