Solcurity: 合约代码安全建议

Solidity智能合约有关安全和代码质量标准的建议，在 BoringCrypto, Mudit Gupta, Runtime Verification, 和 ConsenSys Diligence 的工作基础上整理。

常规审查方法

• 阅读项目的文档、规范和白皮书，了解智能合约的作用。
• 在查看代码之前，先构建一个期望中的合约架构模型。
• 快速浏览一遍合约，感受项目架构，可以利用Surya这类工具。
• 将项目架构与期望中的合约架构模型进行比较，仔细检查不符合预期的部分。
• 创建威胁模型并列出理论上的高级攻击向量。
• 查看与价值交换相关的地方，尤其是transfer，transferFrom，send，call，delegatecall，和 selfdestruct。优先检查它们，确保安全。
• 查看与外部合约交互的地方，并确保所有关于它们的假设都是有效的，例如价格只会上涨等等。
• 对合约进行一般性的逐行审查。
• 从威胁模型中每个参与者的角度进行另一次审查。
• 快速浏览项目的测试 + 代码覆盖率，并深入了解缺乏覆盖率的地方。
• 运行 Slither/Solhint 等工具并审查其输出。
• 查看相关项目及其审计，以检查是否存在任何类似问题或疏忽。

变量（Variable）

• V1 - 变量可以是 internal 吗？
• V2 - 变量可以是 constant 吗？
• V3 - 变量可以是 immutable 吗？
• V4 - 是否设置了可见性？ (SWC-108)
• V5 - 变量的用途和其他重要信息是否使用 natspec 标准记录？
• V6 -可以与相邻的存储变量一起打包吗？
• V7 - 是否可以和其他变量打包在一个struct中？
• V8 - 使用完整的 256 位类型，除非与其他变量一起打包。
• V9 - 如果它是一个public array，是否提供了一个单独的函数来返回完整的数组？
• V10 - 使用更加灵活的internal，而不是private，除非有意阻止子合约访问变量。

结构体（Struct）

• S1 - 是否有必要用struct ？ 可以仅用原始存储变量打包吗？
• S2 - 字段是否打包在了一起 (如果可以) ？
• S3 - struct的用途和所有字段是否使用 natspec 标准进行文档记录 ？

函数（Function）

• F1 - 函数可以是 external 吗？

• F2 - 函数是否应该是 internal ？

• F3 - 函数是否应该是 payable ？

• F4 - 是否可以与另一个类似的函数合并？

• F5 - 验证所有参数都在安全范围内，即使该函数只能由受信任的用户调用。

• F6 - 是否遵循 check-before-effect 模式？ (SWC-107)

• F7 - 检查抢跑的可能性，例如授权方法（approve）。 (SWC-114)

• F8 - 是否会遭受恶意的 gas 不足攻击 ？ (SWC-126)

• F9 - 是否应用了正确的修改器，例如 onlyOwner/requiresAuth ？

• F10 - 返回值是否总是有赋值？

• F11 - 在一个函数能够正确运行之前，写下并测试关于状态的不变性检查（invariants）。

• F12 - 写下并测试关于函数运行后的返回值或任何状态变化的不变性检查。

• F13 - 命名函数时要注意，因为人们会根据名称来假设行为。

• F14 - 如果一个函数是故意unsafe（为了省gas等），使用一个不便的名字来引起人们对其风险的注意。

• F15 - 所有参数、返回值、副作用和其他信息是否用natspec文档记录？

• F16 - 如果函数允许对系统中的另一个用户进行操作，不要假设msg.sender是被操作的用户。

• F17 - 如果函数要求合约处于未初始化的状态，请检查一个明确的initialized变量。不要使用owner == address(0)或其他类似的检查作为替代。

• F18 - 只使用private来有意防止子合约调用该函数，为了灵活性，最好使用internal。

• F19 - 在合法（和安全）的情况，可能子合约希望覆盖该函数的行为，则使用virtual。

修改器（Modifier）

• M1 - 是否没有进行存储更新（重入锁除外）？
• M2 - 是否避免了外部调用？
• M3 - 修改器的用途和其他重要信息是否使用 natspec 标准文档记录？

代码

• C1 - 使用SafeMath或solidity 0.8 检查的数学？(SWC-101)

• C2 - 是否有任何存储槽被多次读取？

• C3 - 是否使用了任何可能导致DoS的无界循环/数组？ (SWC-128)

• C4 - 只对长间隔使用block.timestamp。(SWC-116)

• C5 - 不要使用block.number来表示经过的时间。(SWC-116)

• C7 - 尽可能避免委托调用，特别是对外部（即使是可信的）合约。(SWC-112)

• C8 - 在迭代数组时，不要更新其长度。

• C9- 不要使用blockhash()等来实现随机性。(SWC-120)

• C10 - 签名是否用nonce和block.chainid防止重放 (SWC-121)

• C11 - 确保所有签名使用EIP-712。(SWC-117 SWC-122)

• C12 - 如果需要对大于 2个动态类型进行 hash 时，一般情况下，最好使用abi.encode()而不是abi.encodePacked()的输出进行 hash。(SWC-133)

• C13 - 谨慎使用汇编，不要使用任何任意数据。(SWC-127)

• C14 - 不要假设一个特定的ETH余额. (SWC-132)

• C15 - 避免gas不足攻击。(SWC-126)

• C16 - 私有数据并不是私有的。(SWC-136)

• C17 - 在内存中更新结构体/数组不会在存储中修改它。

• C18 - 永远不要覆盖（shadow）状态变量。(SWC-119)

• C19 - 不要修改函数参数的值。

• C20 - 即时计算一个值是否比存储它更便宜？

• C21 - 所有的状态变量是否从正确的合约中读取（主合约与克隆合约）？

• C22 - 是否正确使用比较运算符（>, <, >=, <=），特别是防止缺位错误（off-by-one error）？

• C23 - 是否正确使用逻辑运算符(==, !=, &&, ||, !), 特别是防止缺位错误（off-by-one error）?

• C24 - 总是先乘后除，除非乘法可能溢出。

• C25 - 魔术数字是否由一个具有直观名称的常数代替？

• C26 - 如果ETH的接收者有一个fallback 函数，会不会造成DoS 攻击？ (SWC-113)

• C27 - 使用openzepplin SafeERC20或安全检查返回值 。

• C28 - 不要在循环中使用msg.value。

• C29 - 如果可能出现递归委托调用，不要使用msg.value（比如合约继承了Multicall/Batchable）。

• C30 - 不要假设msg.sender总是一个相关的用户。

• C31 - 不要使用assert()，除非用于模糊处理或形式验证。(SWC-110)

• C32 - 不要使用tx.origin进行授权。(SWC-115)

• C33 - 不要使用address.transfer()或address.send()。使用.call.value(...)("")代替。(SWC-134)

• C34 - 当使用低级调用时，确保调用前合约存在。

• C35 - 当调用一个有许多参数的函数时，使用命名参数的语法。

• C36 - 不要使用汇编来create2。更倾向于使用新式加salt合约创建语法。

• C37 - 不要使用汇编来访问chainid或合约代码/大小/哈希。更倾向于新式Solidity语法。

• C38 - 当设置一个变量为零值时（0，false，""等），使用delete关键字。

• C39 - 尽可能多地注释 "为什么" 要这样做。

• C40 - 如果使用晦涩的语法或编写非常规的代码，则注释在做 "什么"。

• C41 - 在复杂和定点数学运算的旁边注释解释+输入/输出的例子。

• C42 - 在做了优化的地方做注释，并估计它们能节省多少gas。

• C43 - 在特意避免某些优化的地方进行注释，并估计如果实施这些优化会/不会节省多少gas。

• C44 - 在溢出/下溢是不可能的，或者溢出/下溢在人类的时间尺度上是不现实的（计数器等），使用unchecked块。在使用unchecked的地方进行注释，同时估计它能节省多少gas（如果相关）。

• C45 - 不要依赖Solidity的算术运算符优先级规则。括号不仅用来覆盖默认运算符优先级，而且可以用于强调它。

• C46 - 传递给逻辑/比较运算符（&&/||/>=/=/等）的表达式不应该有副作用。

• C47 - 凡是进行可能导致精度损失的算术运算，都要确保它对系统中的正确角色有利，并用注释记录下来。

• C48 - 用注释来记录为什么必须使用重入锁的原因， 不管是行内或@dev 来记录 。

• C49 - 如果模糊函数仅支持特定范围的参数，使用取模操作限制参数输入范围（如x = x % 10000 + 1来限制在从1到10,000）。

• C50 - 尽可能使用三元表达式来简化分支逻辑。

• C51 - 当在一个以上的地址上操作时，问自己如果它们是相同的会怎样。

外部调用

• X1 -是否真的需要外部合约调用？
• X2 - 如果运行出现报错，是否会导致 DoS？比如 balanceOf() 回退。 (SWC-113)
• X3 - 如果调用重新进入当前函数是否有害？
• X4 - 如果调用重新进入另一个函数是否有害？
• X5 - 是否检查结果并处理了错误? (SWC-104)
• X6 - 如果用完所有提供的 gas 会发生什么?
• X7 - 如果它返回大量数据，会导致调用合约中的 out-of-gas 出错吗？
• X8 - 如果你调用特定函数时返回了 success，也不意味着该函数存在 (phantom functions) 。

静态调用（Static Call）

• S1 -是否真的需要外部合约调用？
• S2 - 在接口中是否应该标记为 view 吗？
• S3 - 如果运行出现报错，是否会导致 DoS？比如 balanceOf() 回退。 (SWC-113)
• S4 - 如果调用进入无限循环，是否会导致 DoS？

事件（Event）

• E1 - 是否应该对任何字段进行索引indexed ？
• E2 - 相关动作的创建者是否包括在索引字段中 ？
• E3 - 不要将包括string 和 bytes 的动态变量设为事件的 inedex。
• E4 - 触发事件的时机和事件变量是否使用 natspec 标准记录文档?
• E5 - 触发事件的函数中所有被操作用户/ID是否设为indexed字段？
• E6 - 避免函数调用和事件参数中使用表达式求值，他们的求值顺序是不可预测的。

合约

• T1 - 使用 SPDX 许可证标识符.

• T2 - 是否所有会修改 storage 变量的函数都触发了事件？

• T3 - 检查所有的继承是否正确，保证他们简洁且线性。 (SWC-125)

• T4 - 如果合约要接收 ETH， 使用 receive() external payable 函数。

• T5 - 写下并测试关于关联存储变量的不变性。

• T6 - 合约的目的和与其他合约的交互是否使用 natspec 标准记录文档？

• T7 - 如果另一个合约必须继承一个合约以解锁其全部功能，则否则应标记为 abstract。

• T8 - 如果构造函数中设置了非不可变量（non-immutable）的值，或在其他函数中被改变，都应该触发事件。

• T9 - 避免过度继承，因为它使得事情复杂化并可能鼓励过度抽象。

• T10 - 始终使用命名的导入语法来明确声明哪些合约是从另一个文件中导入的。

• T11 - 按文件夹/包将引入进行分组，每组之间空一行，外部依赖组放在开头，然后是模拟/测试合约（如有），最后是本地导入。

• T12 - 使用 natspec 标准中的 @notice 记录合约的目的和功能，@dev 记录合约如何与项目内部/外部的其他合约交互。

项目

• P1 - 使用正确的许可证书 （例如如果你依赖GPL协议包，你也要用GPL）。
• P2 - 单元测试所有内容。
• P3 - 尽可能多的模糊测试。
• P4 - 尽可能的使用符号执行。
• P5 - 运行 Slither/Solhint 并审查所有发现。

DeFi

• D1 - 检查你对其他合约作用和返回值的假设。
• D2 - 不要混淆内部计算值与账户实际余额。
• D3 - 不要将 AMM 的现货价格用作价格预言机。
• D4 - 如果没达到链下或预言机的价格目标，不要在 AMM 上进行交易。
• D5 - 使用完备的检查来防止预言机/价格操纵。
• D6 - 要注意 rebasing 代币。如果它们不受支持，要在文档中明确说明。
• D7 - 要注意 ERC-777 代币，即使是你信任的代币也可以被重入。
• D8 - 要注意转账收税的代币，如果它们不受支持，要在文档中明确说明。
• D9 - 要注意使用太大或太小 decimial 的代币，要在文档中明确支持decimial 的最大值和最小值。
• D10 - 要注意依赖原始代币余额来确定收益的合约，直接向合约发送资产，可能会打乱依靠地址的原始以太或代币余额的价格计算功能。
• D11 - 如果你的合约是代币授权的目标地址，请不要根据用户输入随意调用。

原文： https://github.com/transmissions11/solcurity