当前位置: 玩币族首页 > 区块链资产 > Compound阿尔法治理系统审计

Compound阿尔法治理系统审计

2020-06-10 DeFi传教士来源：区块链网络

Compound Finance是一个部署在以太坊网络上的协议，用于自动化，无需许可的借贷ETH和丰富的ERC20代币。这是一个在生态系统中非常广泛使用的去中心化金融系统。

审计历史和当前的范围

在本次审计中，我们观察Compound阿尔法版本的治理系统以及它相关联的COMP代币。虽然初始审计提交的版本号是6858417c91921208c0b3ff342b11065c09665b1b,后来Compound团队提供了后续提交的版本f5976a8a1dcf4e14e435e5581bade8ef6b5d38ea，它修复了一些在初始提交版本识别的一些问题。审计的范围包括了新引进的Compound治理代币（COMP)和治理的阿尔法智能合约。

目前为止我们已审计了几个阶段的Compound智能合约。每个阶段的简单总结包括以下方面。
1.一个子集的Compound智能合约在Compound公共仓库的f385d71983ae5c5799faae9b2dfea43e5cf75262提交版本。阅读报告总结。
2.一个补丁引入了针对危急管理功能的时间延迟和暂停其他功能的能力。这个补丁展示在Compound公共仓库的提交版本681833a557a282fba5441b7d49edb05153bb28ec。阅读这个报告。
3.一次核心CToken智能合约的重构，目的是为标的代币有可能在转移代币时抽取手续费（如USDT).这个重构展示在Compound私有仓库的提交版本2535734126c7c26e9bc452f27f45c5408acff71f。这个报告目前未公开。
4.Compound私有仓库的提交版本2535734126c7c26e9bc452f27f45c5408acff71f和公共仓库的提交版本bcf0bc7b00e289f9b661a0ae934626e018188040之间的代码差异。这些变化引入了处理标的ERC20代币的能力，实现了可升级(如DAI)。这个报告目前未公开。
5.Compound公共仓库提交版本bcf0bc7b00e289f9b661a0ae934626e018188040和9ea64ddd166a78b264ba8006f688880085eeed13之间的差异。审计包含了JumpRateModel智能合约的修改和新加入的两个文件CDaiDelegate.sol以及DAIInterestRateModel.sol。这个报告目前未公开。

修改的高级概览

审计代码的修改引入了一个新的Compound治理系统，包括了Compound治理代币（COMP），还有阿尔法版本的核心治理合约，分别在审计的Comp和GovernorAlpha智能合约中实现。这个系统将最终全部替代Compound的管理员，到时通过Timelock智能合约的时间锁机制授权触发协议中的敏感修改。因为这还是治理系统的初步版本，GovernorAlpha智能合约由高权利的账号管理，这个账号叫guardian,它的权利有：
* 通过cancel函数取消任何未执行的提案
* 任意绕过治理机制去修改Timelock的管理（调用GovernorAlpha智能合约的__queueSetTimelockPendingAdmin和__executeSetTimelockPendingAdmin函数)
* 治理的guardian可以推翻任何排队等候的提案，这些提案尝试修改Timelock智能合约的管理（例如调用setPendingAdmin的提案)，guardian可以调用__queueSetTimelockPendingAdmin不管他们时候的决定。
* 让GovernorAlpha智能合约通过调用__acceptAdmin函数接受管理权限超越Timelock
* 通过调用__abdicate函数废弃。

治理系统的新管理员是临时的，一旦系统达到更稳定的阶段预期是要废弃掉的。
Compound治理代币打算成为一个标准的ERC20代币，有扩展能力让代币持有者授权它们的投票，以及在给定的区块号上查询账号总共的投票。
Compound治理阿尔法合约允许账号达到一定的投票门槛才能提交提案。这些可以由授权的投票持有人进行投票。足够投票量的提案可以在Compound的Timelock智能合约内排队和执行。Compound的治理系统与Timelock智能合约密切协作推送提案交易到Timelock,同时有guardian进行监督。

接下来我们按重要程度展示我们的发现。

严重风险

无

高风险

[H01]通过的提案可能无法排队，取消或者执行
GovernorAlpha智能合约的propose函数允许提议者提交无限数量操作的提案。特别地，该函数对作为参数传递的数组中的元素数量没有硬性限制（例如，targets,values,signatures以及calldatas)
因此，有大量操作并已通过的提案肯能无法排队，取消或执行。这是因为有这么个事实，queue, cancel和execute函数迭代提案的无边界targets数组，这依赖于操作类型的数量，将导致无法预测的超出gas燃料的错误。
所以为避免通过的提案的异常错误，可以考虑设置一个操作数量的硬性上限。

[H02]拥有重复操作的队列提案可能无法执行
GovernorAlpha智能合约允许重复操作的提案被提出和排队。提案中两个或更多操作可能有相同组合的target,value,signature和data数据。
假设一个有重复操作的提案被治理系统通过，然后每个提案中每个操作通过调用queueTransaction函数被分别的排队在Timelock的合约。所有队列的操作保留在Timelock智能合约的queuedTransactions映射中以便未来执行。虽然每个操作通过它target,value,signature,data和eta值进行keccak256哈希被识别，它必须要被注意的是相同提案的所有的操作共享同样的eta。因此，重复操作总是会产生相同识别符的哈希，所以单个的条目将创建在queuedTransactions映射里。
当时间锁超时，提案中全部操作的集合原子性的执行。换句话说，如果其中一个操作失败，全部的提案肯定是失败的。为了执行一个提案，任何人都可以调用GovernorAlpha智能合约的execute函数。这将为每个操作转动调用TimeLock智能合约的executeTransaction函数。考虑到拥有重复操作的提案，首先它们被正常执行并且queuedTransactions映射中的条目将设置为false.然而，第二个重复的操作将和第一个操作共享相同识别符的哈希。结果，由于Timelock第84行中的require语句，它的执行将不可避免地失败，从而回滚整个提案的执行。
可以考虑修改提案中的每个操作的标识方式，以避免它们的标识符发生冲突。这就要允许提案中每个操作被单独标识，进而使Compound治理系统执行包含重复操作的队列提案。
更新：已在随后的提交版本f5976a8a1dcf4e14e435e5581bade8ef6b5d38ea修复，这引入了一个变化，明确地驳回有重复操作的提案在Timelock智能合约中排队。

中风险

[M01]GovernorAlpha合约未能完全匹配规范
* 规范中提到的proposalApproved(uint256): bool 函数未实现。
* 根据规范，提案只有在其他条件下才能成功，“投票数大于法定人数阈值”。然而，当赞成的票数等于或大于法定人数阈值时，该提案被认为是成功的。
* 根据规范，GovernorAlpha智能合约应该有最大数量的提案所包含的操作。然而，已审计的实现没有加以任何操作的数量数量限制（查看propose函数).这让提案者提交的提案完全无法排队，取消或执行（已在问题[H01]通过的提案可能无法排队，取消或者执行解释过了)
可以考虑应用必要的代码修改或是规范修改以达到完全匹配。如果有任何特意的偏差，可以考虑使用文档字符串和内联注释显式地记录它。

[M02]缺乏前期运行减轻ERC20许可的操作
Comp智能合约是一个EERC20代币的合约，继承了EIP20Interface接口。然而，它并没有实现函数去减轻已知的ERC20前期运行许可问题。这意味着每位代币持有人许可代币给其它账号时可能会受到前期运行的潜在攻击。
可以考虑实现函数安全地增加或减少许可的数量。参考OpenZeppelin的ERC20业界标准实现中的函数increaseAllowance和decreaseAllowance。

[M03]提案执行未处理返回数据
GovernorAlpha智能合约的公开execute函数允许任何人执行队列的提案。包含在提案中的每个操作将会触发调用Timelock智能合约的executeTransaction函数。executeTransaction函数返回一个bytes值，包含了调用目标地址返回的任何数据。最需要注意的是这个数据不会记录到发送出来的ExecuteTransaction事件里，因此它应该被调用者处理以避免丢失。然而，这个返回的数据未被GovernorAlpha智能合约的execute函数处理。结果，提案操作相关的返回数据可能就丢失了。
可以考虑通过随后调用executeTransaction函数来处理返回数据。潜在的操作方向可以在包含数据的事件中分析或者以bytes数组值形式返回它给execute函数的调用者。

低风险

[L01]缺乏事件中索引参数
GovernorAlpha智能合约定义的事件中没有参数作索引。可以考虑索引事件的参数避免阻碍链下服务特殊事件搜索和过滤的任务。

[L02]在require语句中修改存储
Comp.sol第143行内的require语句中，签名者的nonce在与给定的nonce比较后是立即递增的。换句话说，假如在增加一个前给定的nonce不同于noces映射存储的，require语句将会失败。然而，这种微妙的语言可能不会被所有的读者捕捉到，可能将在未来修改基础代码时导致混淆或者错误。
为支持可读性，可以考虑在提到的require语句外增加nonce,正确的方式在它被验证后。

[L03]缺失文档字符串
GovernorAlpha智能合约的所有函数缺少文档。这阻碍了审查者理解代码的含义，这不仅是评估安全，也是评估正确性的基础。另外，文档字符串提高了可读性和易维护性。他们应该在可能失败的场景、允许调用它们的角色、返回的值和发出的事件中明确解释函数的含义或者目的。
可以考虑完全文档化智能合约公共API部分的所有函数。实现敏感功能的函数，即使未公开，也应该有清晰的文档。当写文档字符串时，可以考虑遵循以太坊原生规范格式（NatSpect）。

[L04]缺乏输入校验
* Comp智能合约的approve函数不能保证spender地址不为零。查看OpenZeppelin的实现进行参考。
* GovernorAlpha智能合约的propose函数允许description参数为空。
可以考虑实现require语句适当验证所有用户控制的输入。

[L05]有返回语句的函数未声明返回类型
Comp智能合约的公共函数delegate和delegateBySig在它们执行的最后包含了一个返回语句，但是他们在定义中没有明确声明返回类型。此外，两个函数均尝试返回内部_delegate函数的结果，这既未声明返回类型也没返回任何值。
可以考虑删除delegate和delegateBySig函数的return语句，保留两种情况下对内部_delegate函数的调用。

[L06]ERC20代币转移中未文档化，未测试，以及自定义的行为
当Comp代币的transfer和transferFrom功能被调用时，它们内部调用_transferTokens函数。这个内部函数可以执行额外的操作，这些操作不是ERC20标准的一部分。尤其是如果源和目的有不同的委托注册，_decreaseVotes和_increaseVotes函数会执行。这意味着在转移代币是，委托投票数量可能会被更新。
而对于Compound治理系统来说，描述自定义的行为是基本的，但发现没有文档化也没有测试。
可以考虑在transfer和transferFrom函数的文档字符串中明确解释委托投票可能会被更新。此外，可以考虑增加CompTest.js相关的单元测试以保证敏感的功能按照预期工作。

注意&额外信息

[N01]缺失单位
为避免未来更改基础代码是出错，可以考虑行内注释清晰注明状态以衡量votingDelay的单位。

[N02]未明确定义最大许可
为提高可读性，可以考虑定义一个常量MAX_ALLOWANCE_AMOUNT 或 MAX_UINT256 用在Comp智能合约的transferFrom函数，以取代uint(-1)。

[N03]定义uint作为uint256
为更明确，uint的所有实例应该被声明为uint256。

[N04]不一致的编码风格
有少量来自Compound编码风格的差异。特别是：
* GovernorAlpha智能合约__acceptAdmin, __abdicate, __queueSetTimelockPendingAdmin 和 __executeSetTimelockPendingAdmin公共函数没必要预留的双下划线。
* GovernorAlpha.sol第234行的if语句丢失打开和关闭花括号。
* GovernorAlpha和Comp智能合约的内部函数getChainId应该预留下划线明确表明它们的可见性。
为提高可读性，可以考虑在整个基础代码中遵循一致的编码风格。

[N05]命名
* GovernorAlpha智能合约的__acceptAdmin函数应该重命名为__acceptTimelockAdmin。
* GovernorAlpha智能合约的NewProposal事件应该重命名为ProposalCreated以保持与其他相关提案事件的一致性（例如ProposalCanceled, ProposalQueued 和 ProposalExecuted)。

[N06]错别字
在Comp.sol文件中：
* 33行应该指明each account's 来替代 each accounts。
* 57和60行指明that's 来替代 thats。
* 57行指明its delegate 来替代 their delegate。

在README.md文件中：
* Copmtroller应该指明“Contracts”章节的“Governor Alpha”段落的Comptroller。

[N07]未文档化使用uint96类型
Comp智能合约定义了一个Checkpoint结构代表标记账号在给定区块中投票数量的每个检查点。这个结构声明了投票数量作为uint96类型，有效地打包结构数据到128位。然而，这个参数未在代码文档中。注意uint96类型也用于其他一些地方，例如GovernorAlpha智能合约的Receipt结构，同样也有Comp智能合约的balances, allowances以及checkpoints。
可以考虑明确用行内注释来文档化不寻常Solidity类型使用，使代码更容易明白，因此增加工程的可读性。

[N08]投票周期假定区块的频率来计算时间
根据GovernanceAlpha智能合约，投票的周期期望在最近的17280个区块，给定的当前区块时间（平均15秒),将计算成将近3天。投票周期持续的区块数当前是硬编码的，不能通过任何手段修改。然而，已在的以太坊“难度炸弹”可能增加挖矿难度变得更难，因此会增加平均区块时间（查看Etherscan平均区块时间参考).所以，投票周期最终会越来越长超出预期。
可以考虑在GovernorAlpha智能合约增加必要的逻辑让投票的周期时间，当有需要时可以通过治理提案调整。

[N09]冗余的布尔检查
GovernorAlpha.sol的第234行显式比较一个布尔值为true.这是个个冗余的操作，因为结果已经等于布尔值本身。可以考虑删除这个冗余的比较。

[N10]VoteCast事件未记录到投票人的地址
每次投票人为提案投选他们的投票时会调用castVote或castVoteBySig函数，VoteCast事件会发送出去。然而，这个事件当前没有记录投票人的地址，因而阻碍了投票人链下去跟踪投票。
可以考虑在VoateCast事件中记录投票人的地址。

[N11]智能合约不兼容solc 0.5.12
Comp和GovernorAlpha智能合约指定了编译版本大于等于solc 0.5.12。然而，看到以下的输出，两个合约通过solc 0.5.12编译失败。

$ solc --version
solc, the solidity compiler commandline interface
Version: 0.5.12+commit.7709ece9.Linux.g++

$ solc --allow-paths . --evm-version istanbul Governance/*.sol
Governance/Comp.sol:2:1: Warning: Experimental features are turned on. Do not use experimental features on live deployments.
pragma experimental ABIEncoderV2;
^-------------------------------^
Governance/GovernorAlpha.sol:2:1: Warning: Experimental features are turned on. Do not use experimental features on live deployments.
pragma experimental ABIEncoderV2;
^-------------------------------^
Governance/Comp.sol:282:20: Error: Variable not found or variable not lvalue.
assembly { chainId := chainid() }
^-----^
Governance/GovernorAlpha.sol:299:20: Error: Variable not found or variable not lvalue.
assembly { chainId := chainid() }
^-----^

可以考虑只允许编译版本大于0.5.12

[N12]校验提案状态的不一致风格
GovernorAlpha智能合约cancel函数的校验提案状态的风格应该简化以提高与execute和queue函数相似状态校验的一致性。特别是，给定的state本地变量未在后续的cancel函数使用，state函数可以被内部的require语句调用。

[N13]require语句中不正确的错误消息
两个require语句包含了不正确的错误消息。特别是：
* GovernorAlpha.sol的258行：castVate应该说是_castVote。
* GovernorAlpha.sol的261行：castVate应该说是_castVote。
可以考虑修复这些错误消息以避免排除故障时的混淆。

总结

没有严重风险但两个高风险问题被发现。一些修改已提出以遵循最佳实践和减少潜在的攻击面。

原文链接：https://blog.openzeppelin.com/compound-alpha-governance-system-audit

最后提醒一下，市场有风险，本文只是个研究，不作为投资建议，请合理控制风险。

点赞就是对传教士最大的鼓励，谢谢支持。

—-

编译者/作者：DeFi传教士

玩币族申明：玩币族作为开放的资讯翻译/分享平台，所提供的所有资讯仅代表作者个人观点，与玩币族平台立场无关，且不构成任何投资理财建议。文章版权归原作者所有。

知识合约智能合约代币区块

Compound阿尔法治理系统审计

相关阅读：

新文章

知识推荐