2021 年 12 月 6 日
概要
タリー チームは、共通のガバナンス契約を改善し、より柔軟性を持たせることを最終目標として、一連の契約をレビューおよび監査するように依頼しました。 コードを確認し、結果を公開しました。
システム概要
システムは、次のXNUMXつの主要な契約に依存しています。
- A
SafeGuard
契約テンプレート。 この契約はadmin
のTimelock
契約し、モジュール式の役割のレイヤーを追加しますTimelock
のアクション。 このコントラクトは、プロポーザルの状態(キューイング、キャンセル、および実行)に対して個別の責任とアクセス権を持ついくつかの役割を定義します。 - A
SafeGuardFactory
契約は新しいを展開しますSafeGuard
および対応する新しいTimelock
契約する。 次に、内部にタイムロックアドレスを設定しますSafeGuard
契約し、新しいを登録しますSafeGuard
にRegistry
. - A
Registry
展開されたリストを保持しますSafeGuards
対応する バージョン番号.
SafeGuardFactory
基本的にスポーンします 新製品 SafeGuard
それが呼び出されるときはいつでも。 A SafeGuard
ラップアラウンドです Timelock
操作 これにより、アクションごとに異なる個別の役割が追加されます。 役割は、OpenZeppelinを使用して構成されています AccessControlEnumerable
複数のアドレスが同じ共有の役割を持つことができるマルチシグウォレットとビジネスユースケースに、より柔軟性と互換性を与える契約。
アップデート: PR#10、タリーチームは削除することを決定しました Registry
契約する。 展開されたセーフガードのリストは、 SafeGuardFactory
契約、 safeGuards
列挙可能なセットと、に保存されているバージョン safeGuardVersion
マッピング。
役割
SafeGuard
契約では、次の役割が定義されています。
アップデート: CREATOR_ROLE
問題L02の修正として、役割が削除されました。
対象領域
コミットを監査しました b2c63a9dfc4090be13320d999e7c6c1d842625d3
safeguard
リポジトリ。 範囲内にスマートコントラクトがあります contracts
ディレクトリ。 しかし mocks
ディレクトリは範囲外と見なされました。
仮定
システムはアップグレード可能ではありません。 The Registry
アドレスが設定されています コンストラクターで SafeGuardFactory
そしてそれぞれ SafeGuard
持つことができます Timelock
一度だけ設定。 これは、新しい場合 Registry
新しく展開されます SafeGuardFactory
展開する必要もあります。 の場合 Timelock
実装の変更、古い SafeGuard
sは廃止され、新しいものを展開する必要があります。
さらに、システムは、 Timelock
縮小することはできません。 これは、この監査の範囲外と見なされました。 チームはまだ実装を完了していません Timelock
契約する。 このレポートを書いている時点では、 化合物のタイムロックの実装 プロジェクトで使用されています。
コードベースは、XNUMX週間の間にXNUMX人の監査人によって監査されました。ここでは、調査結果を示します。
重大な重大度
なし。
重大度が高い
[H01]ETHは内部にロックできます Timelock
縮小することはできません。
Tally
チームは元々、 GovernorBravoDelegate
複合契約。
この監査の過程で、 Tally
チームが発見した 制限 ETHが直接送信されたコンパウンドのガバナーで Timelock
ガバナンスの提案では使用できません。永続的にスタックしているわけではありませんが、複雑な回避策を取得する必要があります。
これは、ガバナーの実装では、提案のすべての価値を次のように添付する必要があるためです。 msg.value
実行をトリガーするアカウントによって、いかなる方法でも使用しない Timelock
ETHファンド。
同じ問題が後で特定されました SafeGuard
実装 チームは問題を認識しており、修正中です。
問題を修正する際は、このアプローチの使用を検討してください 同じ問題のためにOpenZeppelinライブラリによって採用されました.
アップデート: コミットで修正 7337db227edda83533be586135d96ddac4f5bf29
.
[H02]SafeGuardFactoryをフリーズできます
Registry
契約は、すべての SafeGuards
その SafeGuardFactory
生産する。 それは外部を持っています register
function これはこの目的で使用されます。
同時に、 SafeGuardFactory
コンストラクターに、ローカルの割り当てがあります registry
入力値への値。 の値を変更する可能性はありません registry
可変であり、このため、新しい場合 Registry
デプロイされると、新しいファクトリもデプロイする必要があります。
SafeGuardFactory
があります createSafeGuard
function、最初の担当 新しい展開 SafeGuard
をタップし、その後、 新しい Timelock
のアドレスで SafeGuard
as admin
, 次に、 timelock
の変数 SafeGuard
契約そして最後に 登録する SafeGuard
レジストリ内.
問題は、 createSafeGuard
新しいの決定論的アドレスを直接登録できる攻撃者によって強制的に失敗させることができます SafeGuard
その作成前。 契約があるときはいつでも を作成します new
、そのナンスが増加し、契約の新しいインスタンスが展開される場所のアドレスは、元の契約アドレスとそのナンスによって決定できます。 したがって、攻撃者は新しいアドレスの多くを事前に計算できます SafeGuards
展開され、それらのアドレスをに登録します Registry
を呼び出すことによって register
働き。 これにより、 createSafeGuard
〜へ 元に戻す から Registry
すでにアドレスが含まれています。
外部のアクターが公に電話をかけるのを避けるために Register
契約、アクセスを制限することを検討してください register
によって排他的に呼び出しを受け入れる機能 SafeGuardFactory
.
アップデート: で修正されました PR#10。 タリーチームは Registry
契約する。
中程度の重大度
なし。
重大度が低い
[L01]コメントアウトされたコード
Registry
契約には以下が含まれます コメント化されたコード行。 読みやすさを向上させるために、コードベースから削除することを検討してください。
アップデート: で修正されました PR#10 コミットします 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L02] SafeGuardの管理者は、作成者の役割を任意のアドレスに割り当てることができます
SafeGuard
契約はの役割を定義します CREATOR_ROLE
名前が示すように、これはセーフガードの作成者に割り当てられます。
ただし、呼び出すことによって grantRole
の機能 AccessControlEnumerable
縮小することはできません。 OpenZeppelinコントラクトライブラリでは、管理者はこの役割を任意のアドレスに付与できます。 これは混乱を引き起こす可能性があります の作成者 SafeGuard
することができます SafeGuardFactory
.
コードベース全体で、この役割はユーザーが setTimelock
function SafeGuard
契約する。 設計上、システムは次のことを保証します setTimelock
function 一度だけ呼び出すことができます、から 中で SafeGuardFactory
縮小することはできません。.
削除することを検討してください CREATOR_ROLE
からの役割 SafeGuard
契約と使用 onlyOwner
修飾子 セクションに setTimelock
機能。
アップデート: で修正されました PR#10.
[L03]インターフェイスの定義と実装が正しくありません
ISafeGuard
インターフェイスは定義しません queueTransactionWithDescription
function に実装されています SafeGuard
契約、そして同時に、それは定義します __abdicate、__ queueSetTimelockPendingAdmin、および__executeSetTimelockPendingAdmin 機能しますが、実装されていません。
コードベースの正確性と一貫性を向上させるには、リファクタリングを検討してください。 ISafeGuard
完全に一致するインターフェイス SafeGuard
インプリメンテーション。
アップデート: コミットで修正 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L04] docstringがありません
コードベースの一部のコントラクトと関数にはドキュメントがありません。 例えば、 いくつかの機能 セクションに SafeGuard
契約する。
さらに、一部のdocstringは、次のような非公式な言語を使用します。 上記 setTimelock
内の関数 SafeGuard
縮小することはできません。.
これは、セキュリティだけでなく正確さも正しく評価するための基本であるコードの意図についてのレビューアの理解を妨げます。 さらに、docstringは読みやすさを向上させ、メンテナンスを容易にします。 関数の目的または意図、関数が失敗する可能性のあるシナリオ、関数の呼び出しが許可されている役割、返される値、および発行されるイベントを明示的に説明する必要があります。
コントラクトのパブリックAPIの一部であるすべての関数(およびそれらのパラメーター)を完全に文書化することを検討してください。 機密性の高い機能を実装する機能は、公開されていなくても、明確に文書化する必要があります。 docstringを作成するときは、次のことを検討してください。 イーサリアムナチュラル仕様フォーマット (NatSpec)。
アップデート: 部分的に修正されました PR#10。 適切なdocstringが、コードベース全体のさまざまな関数に追加されました。 ただし、現在の変更に加えて、次の変更を行うことを検討してください。
- Add
description
として@param
上記のdocstringでqueueTransactionWithDescription
function - Add
@param
セクションに ドキュメント文字列 上記createSafeGuard
の機能SafeGuardFactory
縮小することはできません。 - Add
@return
の関数の上のdocstringsSafeGuardFactory
契約する。
[L05]役に立たないまたは繰り返されるコード
コードベースには、コードが繰り返されるか、不要になる場所があります。 いくつかの例は次のとおりです。
- 行29-32
Registry
契約は役に立たないので、_add
の機能EnumerableSet
縮小することはできません。 すでにこれらのチェックを実行しています すでに設定されている値に対して。 - 行62, 67, 73 & 78
SafeGuard
コントラクトはすべて同じ正確な操作を繰り返しています。 コードの重複を避けるために、内部関数にカプセル化することを検討してください。 - 行62-63 & 67-68 of
SafeGuard
繰り返されます。 それらを単一の内部関数にカプセル化することを検討してください。 - の使用法
gasleft
関数の呼び出しで転送するガスの量を指定しますexecuteTransaction
不要です。 これは、その実行時点で、残ったガス全体が実行を継続するために使用されるためです。 これが明確にするためではない場合は、削除することを検討してくださいgas
呼び出しからのパラメーター。
提案された修正を適用して、よりクリーンなコードを生成し、コードベース全体の一貫性とモジュール性を向上させることを検討してください。
アップデート: で修正されました PR#10 コミットします 7fd27df16fc879d990d36a167a0b6e719e578558
.
メモと追加情報
[N01]一貫性のないスタイル
コードベースには、スタイルの違いが読みやすさに影響を与え、コードを理解しにくくする場所がいくつかあります。 いくつかの例は次のとおりです。
-
Registry
コントラクトは、コントラクト全体でdocstringに異なるスタイルを使用します。 -
SafeGuard
契約がイベントを発行しているのはqueueTransactionWithDescription
が呼び出されますが、トランザクションを処理する他の関数ではイベントは発行されません。 -
SafeGuard
契約、時々 値 名前付きパラメータとして使用され、場合によっては _価値 使用されている。
一貫したコーディングスタイルがプロジェクトの読みやすさに追加する価値を考慮して、Solhintなどのリンターツールを使用して標準のコーディングスタイルを適用することを検討してください。
アップデート: で修正されました PR#10 コミットします 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N02]ライセンスがありません
コードベース内の次のコントラクトには、SPDXライセンス識別子がありません。
コンパイラの警告を消し、コードベース全体の一貫性を高めるには、ライセンス識別子を追加することを検討してください。 それをしている間、参照することを検討してください spdx.dev ガイドライン。
アップデート: で修正されました PR#10 コミットします 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N03] OpenZeppelinコントラクトの依存関係が固定されていません
重大な変更が将来のアップデートでリリースされた場合の予期しない動作を防ぐために OpenZeppelin契約のライブラリ、この依存関係のバージョンをに固定することを検討してください package.json ファイルにソフトウェアを指定する必要があります。
アップデート: で修正されました PR#10.
[N04]Solidityコンパイラのバージョンが固定されていません
コードベース全体で、のバージョンを固定することを検討してください Solidityコンパイラ 最新の安定バージョンに。 これにより、互換性のない将来のリリースによる予期しないバグの発生を防ぐことができます。 特定のバージョンを選択するには、開発者はプロジェクトに必要なコンパイラの機能と 既知のバグのリスト 各Solidityコンパイラバージョンに関連付けられています。
アップデート: で修正されました PR#10.
[N05]タイプミス
コードベース全体のさまざまなインスタンスで、単語 role
スペルミス rol
。 そのような例のXNUMXつは 内のdocstring constructor
SafeGuard
縮小することはできません。.
コードの可読性を向上させるために、これらのタイプミスを修正することを検討してください。
アップデート: 部分的に修正されました PR#10。 のつづりが role
修正されたコメント「管理者の役割を定義された管理者アドレスに設定する」は、「管理者の役割を定義された管理者アドレスに設定する」である必要があります。 さらに、「実行」のつづりが間違っています。 SafeGuard
契約 ライン69, ライン82, ライン96 & ライン110 「利用可能」のつづりが間違っている ライン70, ライン83, ライン97, ライン111。 また、次のような非公式な単語を置き換えることを検討してください 「つもり」 in SafeGuard
「行く」などの正式な代替案との契約。
[N06]uintをuint256として宣言します
コードベースには、変数が宣言されている場所がいくつかあります。 uint
代わりにデータ型 uint256
。 例えば、 eta
の変数 QueueTransactionWithDescription
イベント SafeGuard
契約する。
明示性を優先するために、 uint
として宣言する必要があります uint256
.
アップデート: で修正されました PR#10 コミットします 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N07]未使用のインポート
SafeGuard
契約は console
契約しますが、決して使用しません。
コードの可読性を向上させるために、未使用のインポートを削除することを検討してください。
アップデート: で修正されました PR#10.
結論
XNUMXつの高い脆弱性と他のいくつかの小さな脆弱性が発見され、推奨事項と修正が提案されています。
- &
- アクセス
- 越えて
- Action
- 行動
- NEW
- 住所
- 管理人
- すべて
- 既に
- しかし
- API
- アプローチ
- 周りに
- 監査
- 基本的に
- さ
- バグ
- ビジネス
- コール
- 例
- 原因となる
- 変化する
- チャージ
- コード
- コーディング
- コマンドと
- 混乱
- 考慮
- 含まれています
- 続ける
- 縮小することはできません。
- 契約
- 可能性
- クリエイター
- 電流プローブ
- データ
- 取引
- 設計
- 開発者
- 異なります
- 発見
- 手の込んだ
- ETH
- イベント
- イベント
- 例
- 工場
- 特徴
- 最後に
- 名
- 修正する
- 柔軟性
- 発見
- function
- 資金
- 未来
- GAS
- 与え
- 目標
- ガバナンス
- 知事
- ガイドライン
- 持って
- 助けます
- こちら
- ハイ
- 認定条件
- HTTPS
- 実装
- 増える
- 増加した
- インタフェース
- IT
- 既知の
- 言語
- 最新の
- 図書館
- ライセンス
- LINE
- リスト
- ローカル
- ロック
- 見
- 作成
- 一致
- モジュラー
- マルチシグ
- その他
- 現在
- プロセス
- プロジェクト
- 提案
- 公共
- パブリッシュ
- 登録
- リリース
- レポート
- 倉庫
- 結果
- レビュー
- セキュリティ
- セッションに
- 設定
- shared
- スマート
- スマート契約
- 固い
- 都道府県
- 介して
- 全体
- 時間
- 豊富なツール群
- 追跡する
- 取引
- 更新版
- us
- users
- 値
- 脆弱性
- 財布
- 週間
- 誰
- 以内
- 言葉
- 書き込み