Diamond标准:好想法但糟糕设计的智能合约升级方案

本文深入分析了EIP-2535 Diamond标准的实现缺陷,包括过度工程化、存储指针风险、函数遮蔽等问题,并提供了升级方案的最佳实践和安全建议。

好想法,坏设计:Diamond标准为何不及格 - Trail of Bits博客

TL;DR: 我们审计了Diamond标准提案的合约可升级性实现,目前无法推荐该方案——但请参考我们的建议和升级策略指南。

我们最近审计了Diamond标准代码的实现,这是一种新的可升级模式。虽然这是值得称赞的尝试,但Diamond提案和实现引发了许多担忧。代码过度工程化,包含大量不必要的复杂性,目前我们不推荐使用。

当然,该提案仍处于草案阶段,有改进空间。一个可行的可升级标准应包含:

  • 清晰简单的实现。标准应易于阅读,以简化与第三方应用的集成
  • 完整的升级程序检查清单。升级是高风险过程,必须详细说明
  • 针对最常见可升级性错误的链上缓解措施,包括函数遮蔽和冲突
  • 相关风险列表。可升级性很困难,可能隐藏安全问题或暗示风险很小
  • 与最常见测试平台集成的测试。测试应突出显示如何部署系统、如何升级新实现以及升级可能失败的方式

遗憾的是,Diamond提案未能解决这些问题。这很可惜,因为我们希望看到一个可升级标准能够解决或至少缓解可升级合约的主要安全陷阱。本质上,标准编写者必须假设开发人员会犯错,并旨在构建一个能够缓解这些问题的标准。

尽管如此,从Diamond提案中仍有很多值得学习的地方。请继续阅读了解:

  • Diamond提案的工作原理
  • 我们的审查发现
  • 我们的建议
  • 可升级性标准最佳实践

Diamond提案范式

Diamond提案是EIP 2535中定义的一个进行中的工作。该草案声称基于delegatecall提出了一种新的合约可升级性范式。(仅供参考,这里概述了可升级性工作原理。)

EIP 2535提议使用:

  • 实现查找表
  • 任意存储指针

查找表

基于delegatecall的可升级性主要使用两个组件——代理和实现:

图1:具有单一实现的基于delegatecall的可升级性

用户与代理交互,代理delegatecall到实现。执行实现代码,而存储保留在代理中。

使用查找表允许delegatecall到多个合约实现,根据要执行的函数选择适当的实现:

图2:具有多个实现的基于delegatecall的可升级性

这种模式并不新颖;其他项目过去曾使用此类查找表实现可升级性。参见ColonyNetwork示例。

任意存储指针

该提案还建议使用Solidity最近引入的功能:任意存储指针,它(如名称所示)允许将存储指针分配到任意位置。

由于存储保留在代理上,实现的存储布局必须遵循代理的存储布局。在进行升级时,跟踪此布局可能很困难(参见此处示例)。

EIP提议每个实现都有一个关联结构来保存实现变量,以及一个指向将存储结构的任意存储位置的指针。这类似于非结构化存储模式,其中新的Solidity功能允许使用结构而不是单个变量。

假设只要它们各自的基础指针不同,来自两个不同实现的两个结构就不会冲突。

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

图3:存储指针示例

图4:存储指针表示

顺便问一下,什么是"diamond"?

EIP 2535引入了"diamond术语",其中单词"diamond"表示代理合约,“facet"表示实现,等等。不清楚为什么引入这种术语,特别是因为可升级性的标准术语是众所周知且定义明确的。如果您阅读该提案,这里有一个关键帮助您翻译:

Diamond词汇 通用名称
Diamond 代理
Facet 实现
Cut 升级
Loupe 委托函数列表
Finished diamond 不可升级
Single cut diamond 移除可升级性功能

图5:Diamond提案使用新术语来指代现有概念

审计发现和建议

我们对diamond实现的审查发现:

  • 代码过度工程化,包含几个 misplaced 优化
  • 使用存储指针有风险
  • 代码库存在函数遮蔽
  • 合约缺乏存在性检查
  • diamond术语增加了不必要的复杂性

过度工程化的代码

虽然EIP中提出的模式很简单,但其实际实现难以阅读和审查,增加了出现问题的可能性。

例如,链上保存的大量数据很麻烦。虽然提案只需要一个从函数签名到实现地址的查找表,但EIP定义了许多需要存储额外数据的接口:

 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 {
    /// 这些函数预计会被工具频繁调用
    
    struct Facet {
        address facetAddress;
        bytes4[] functionSelectors;
    }

    /// @notice 获取所有facet地址及其四字节函数选择器
    /// @return facets_ Facet
    function facets() external view returns (Facet[] memory facets_);

    /// @notice 获取特定facet支持的所有函数选择器
    /// @param _facet facet地址
    /// @return facetFunctionSelectors_
    function facetFunctionSelectors(address _facet) external view returns (bytes4[] memory facetFunctionSelectors_);

    /// @notice 获取diamond使用的所有facet地址
    /// @return facetAddresses_
    function facetAddresses() external view returns (address[] memory facetAddresses_);

    /// @notice 获取支持给定选择器的facet
    /// @dev 如果未找到facet,返回address(0)
    /// @param _functionSelector 函数选择器
    /// @return facetAddress_ facet地址
    function facetAddress(bytes4 _functionSelector) external view returns (address facetAddress_);
}

图6:Diamond接口

这里,facetFunctionSelectors返回实现的所有函数选择器。此信息仅对离线组件有用,这些组件已经可以从合约事件中提取信息。在链上不需要这样的功能,特别是因为它显著增加了代码复杂性。

此外,许多代码复杂性是由于在不需要的地方进行优化造成的。例如,用于升级实现的函数应该很简单。获取新地址和签名,它应该更新查找表中的相应条目。执行此操作的部分函数如下:

 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
// 添加或替换函数
if (newFacet != 0) {
    // 添加和替换选择器
    for (uint selectorIndex; selectorIndex < numSelectors; selectorIndex++) {
        bytes4 selector;
        assembly {
            selector := mload(add(facetCut,position))
        }
        position += 4;
        bytes32 oldFacet = ds.facets[selector];
        // 添加
        if(oldFacet == 0) {
            // 在函数结束时更新最后一个插槽
            slot.updateLastSlot = true;
            ds.facets[selector] = newFacet | bytes32(selectorSlotLength) << 64 | bytes32(selectorSlotsLength);
            // 清除插槽中的选择器位置并添加选择器
            slot.selectorSlot = slot.selectorSlot & ~(CLEAR_SELECTOR_MASK >> selectorSlotLength * 32) | bytes32(selector) >> selectorSlotLength * 32;
            selectorSlotLength++;
            // 如果插槽已满,则写入存储
            if(selectorSlotLength == 8) {
                ds.selectorSlots[selectorSlotsLength] = slot.selectorSlot;
                slot.selectorSlot = 0;
                selectorSlotLength = 0;
                selectorSlotsLength++;
            }
        }
        // 替换
        else {
            require(bytes20(oldFacet) != bytes20(newFacet), "Function cut to same facet.");
            // 替换旧的facet地址
            ds.facets[selector] = oldFacet & CLEAR_ADDRESS_MASK | newFacet;
        }
    }
}

图7:升级函数

为了优化此函数的gas效率付出了很多努力。但是升级合约很少进行,因此无论其gas成本如何,它永远不会是昂贵的操作。

在另一个不必要的复杂性示例中,使用位操作而不是结构:

1
2
3
4
5
6
uint selectorSlotsLength = uint128(slot.originalSelectorSlotsLength);
uint selectorSlotLength = uint128(slot.originalSelectorSlotsLength >> 128);
// uint32 selectorSlotLength, uint32 selectorSlotsLength
// selectorSlotsLength是selectorSlots中32字节插槽的数量
// selectorSlotLength是selectorSlots最后一个插槽中选择器的数量
uint selectorSlotsLength;

图8:使用位操作而不是结构

11月5日更新: 自我们审计以来,参考实现已更改,但其底层复杂性仍然存在。现在有三个参考实现,这对用户来说更加困惑,对提案的进一步审查也更加困难。

我们的建议

  • 始终追求简单,并尽可能将代码保持在链下
  • 编写新标准时,保持代码可读且易于理解
  • 在实施优化之前分析需求

存储指针风险

尽管声称如果基础指针不同就不可能发生冲突,但恶意合约可能与另一个实现的变量冲突。基本上,这是因为Solidity存储变量的方式以及影响映射或数组的方式。例如:

 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
contract TestCollision{
    
    // 该合约代表两个实现A和B
    // A有一个嵌套结构
    // A和B有不同的基础存储指针
    // 然而写入B将导致写入A变量
    // 这是因为B的基础存储指针
    // 与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;
    }
}

图9:存储指针冲突

在exploit中,对B_STORAGE基础指针的写入实际上将写入my_items[0].elems[0],这是从A_STORAGE基础指针读取的。恶意所有者可以推送看起来良性但包含后门的升级。

EIP没有防止这些恶意冲突的指南。此外,如果在删除后重新使用指针,重新使用将导致数据泄露。

我们的建议

  • 低级存储操作有风险,因此在设计依赖它们的系统时要格外小心
  • 使用带有结构的非结构化存储来实现可升级性是一个有趣的想法,但它需要完整的文档和关于在基础指针中检查什么的指南

函数遮蔽

可升级合约通常在代理中有函数遮蔽应该被委托的函数。对这些函数的调用永远不会被委托,因为它们将在代理中执行。此外,相关代码将不可升级。

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

    constructor(...) public{
          // 添加my_targeted_function() 
          // 作为委托函数
    }
    
    function my_targeted_function() public{
    }
 
    fallback () external payable{
          // 委托给实现
    }
}

图10:遮蔽问题的简化

尽管这个问题是众所周知的,并且代码经过了EIP作者的审查,但我们在合约中发现了两个函数遮蔽的实例。

我们的建议

  • 编写可升级合约时,使用crytic.io或slither-check-upgradeability来捕获遮蔽实例
  • 这个问题突出了一个重要点:开发人员会犯错。任何新标准都应包括对常见错误的缓解措施,如果它要比自定义解决方案更好地工作

无合约存在性检查

另一个常见错误是缺少对合约代码的存在性检查。如果代理委托到错误的地址,或已被销毁的实现,对实现的调用将返回成功,即使没有执行任何代码(参见Solidity文档)。因此,调用者不会注意到问题,这种行为可能会破坏第三方合约集成。

 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)}
    }
}

图11:没有合约存在性检查的fallback函数

我们的建议

  • 调用任意合约时始终检查合约存在性
  • 如果gas成本是一个问题,仅在调用不返回数据时执行此检查,因为相反的结果意味着执行了一些代码

不必要的Diamond术语

如前所述,Diamond提案严重依赖其新创建的词汇。这容易出错,使审查更加困难,并且对开发人员没有好处。

diamond是使用其facet中的函数来执行函数调用的合约。diamond可以有一个或多个facet。

facet一词来自钻石行业。它是钻石的一个侧面或平面。钻石可以有许多facet。在这个标准中,facet是一个包含一个或多个函数的合约,执行diamond的功能。

loupe是用于观察钻石的放大镜。在这个标准中,loupe是一个提供函数来查看diamond及其facet的facet。

图12:EIP将标准术语重新定义为与软件工程无关的术语

我们的建议: 使用常见、众所周知的词汇,不需要时不要发明术语

Diamond提案是死胡同吗?

如前所述,我们仍然相信社区将从标准化的可升级模式中受益。但当前的Diamond提案不符合预期的安全要求,也没有比自定义实现带来足够的好处。

然而,该提案仍处于草案阶段,可能会演变成更简单更好的东西。即使没有,使用的一些现有技术,如查找表和任意存储指针,值得继续探索。

那么……可升级性是否可行?

多年来,我们审查了许多可升级合约,并就此主题发表了若干分析。可升级性很困难,容易出错,并增加风险,我们通常仍然不推荐它作为解决方案。但需要在其合约中实现可升级性的开发人员应该:

  • 考虑不需要delegatecall的可升级性设计(参见Gemini实现)
  • 彻底审查现有解决方案及其局限性:
    • 合约升级反模式
    • 合约迁移工作原理
    • 使用OpenZeppelin的可升级性
  • 使用crytic.io,或将slither-check-upgradeability添加到您的CI中

如果您对升级策略有任何疑问,请联系我们。我们随时准备提供帮助!

comments powered by Disqus
使用 Hugo 构建
主题 StackJimmy 设计