Solcurity: 合约代码安全建议

liumuhui 2年前 (2023-06-28) 阅读数 237 #技术

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 - 当在一个以上的地址上操作时，问自己如果它们是相同的会怎样。