Good idea, bad design: How the Diamond standard falls short

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13

bytes32 constant POSITION = keccak256(
    "some_string"
);

struct MyStruct {
    uint var1;
    uint var2;
}

function get_struct() internal pure returns(MyStruct storage ds) {
    bytes32 position = POSITION;
    assembly { ds_slot := position }
}

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27

interface IDiamondLoupe {
    /// These functions are expected to be called frequently
    /// by tools.

    struct Facet {
        address facetAddress;
        bytes4[] functionSelectors;
    }

    /// @notice Gets all facet addresses and their four byte function selectors.
    /// @return facets_ Facet
    function facets() external view returns (Facet[] memory facets_);

    /// @notice Gets all the function selectors supported by a specific facet.
    /// @param _facet The facet address.
    /// @return facetFunctionSelectors_
    function facetFunctionSelectors(address _facet) external view returns (bytes4[] memory facetFunctionSelectors_);

    /// @notice Get all the facet addresses used by a diamond.
    /// @return facetAddresses_
    function facetAddresses() external view returns (address[] memory facetAddresses_);

    /// @notice Gets the facet that supports the given selector.
    /// @dev If facet is not found return address(0).
    /// @param _functionSelector The function selector.
    /// @return facetAddress_ The facet address.
    function facetAddress(bytes4 _functionSelector) external view returns (address facetAddress_);

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34

// adding or replacing functions
if (newFacet != 0) {
    // add and replace selectors
    for (uint selectorIndex; selectorIndex < numSelectors; selectorIndex++) {
        bytes4 selector;
        assembly {
            selector := mload(add(facetCut,position))
        }
        position += 4;
        bytes32 oldFacet = ds.facets[selector];
        // add
        if(oldFacet == 0) {
            // update the last slot at then end of the function
            slot.updateLastSlot = true;
            ds.facets[selector] = newFacet | bytes32(selectorSlotLength) << 64 | bytes32(selectorSlotsLength);
            // clear selector position in slot and add selector
            slot.selectorSlot = slot.selectorSlot & ~(CLEAR_SELECTOR_MASK >> selectorSlotLength * 32) | bytes32(selector) >> selectorSlotLength * 32;
            selectorSlotLength++;
            // if slot is full then write it to storage
            if(selectorSlotLength == 8) {
                ds.selectorSlots[selectorSlotsLength] = slot.selectorSlot;
                slot.selectorSlot = 0;
                selectorSlotLength = 0;
                selectorSlotsLength++;
            }
        }
        // replace
        else {
            require(bytes20(oldFacet) != bytes20(newFacet), "Function cut to same facet.");
            // replace old facet address
            ds.facets[selector] = oldFacet & CLEAR_ADDRESS_MASK | newFacet;
        }
    }
}

1
2
3
4
5
6
7

uint selectorSlotsLength = uint128(slot.originalSelectorSlotsLength);
uint selectorSlotLength = uint128(slot.originalSelectorSlotsLength >> 128);
// uint32 selectorSlotLength, uint32 selectorSlotsLength
// selectorSlotsLength is the number of 32-byte slots in selectorSlots.
// selectorSlotLength is the number of selectors in the last slot of
// selectorSlots.
uint selectorSlotsLength;

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58

contract TestCollision{
    
    // The contract represents two implementations, A and B
    // A has a nested structure 
    // A and B have different bases storage pointer 
    // Yet writing in B, will lead to write in A variable
    // This is because the base storage pointer of B 
    // collides with A.ds.my_items[0].elems
    
    bytes32 constant public A_STORAGE = keccak256(
        "A"
    );
    
    struct InnerStructure{
        uint[] elems;
    }
    
    struct St_A {
        InnerStructure[] my_items;
    }

    function pointer_to_A() internal pure returns(St_A storage s) {
        bytes32 position = A_STORAGE;
        assembly { s_slot := position }
    }
    
    
    bytes32 constant public B_STORAGE = keccak256(
        hex"78c8663007d5434a0acd246a3c741b54aecf2fefff4284f2d3604b72f2649114"
    );
    
    struct St_B {
        uint val;
    }

    function pointer_to_B() internal pure returns(St_B storage s) {
        bytes32 position = B_STORAGE;
        assembly { s_slot := position }
    }
    
    
    constructor() public{
        St_A storage ds = pointer_to_A();
        ds.my_items.push();
        ds.my_items[0].elems.push(100);
    }
    
    function get_balance() view public returns(uint){
        St_A storage ds = pointer_to_A();
        return ds.my_items[0].elems[0];
    }
    
    function exploit(uint new_val) public{
        St_B storage ds = pointer_to_B();
        ds.val = new_val;
    }
    
}

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14

contract Proxy {

    constructor(...) public{
          // add my_targeted_function() 
          // as a delegated function
    }
    
    function my_targeted_function() public{
    }
 
    fallback () external payable{
          // delegate to implementations
    }
}

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16

fallback() external payable {
    DiamondStorage storage ds;
    bytes32 position = DiamondStorageContract.DIAMOND_STORAGE_POSITION;
    assembly { ds_slot := position }
    address facet = address(bytes20(ds.facets[msg.sig]));
    require(facet != address(0), "Function does not exist.");
    assembly {
        calldatacopy(0, 0, calldatasize())
        let result := delegatecall(gas(), facet, 0, calldatasize(), 0, 0)
        let size := returndatasize()
        returndatacopy(0, 0, size)
        switch result
        case 0 {revert(0, size)}
        default {return (0, size)}
    }
}