2021 年 12 月 16 日
概要
1inch チームは私たちにレビューと監査を依頼しました 指値注文プロトコル v2 スマート コントラクト。コードを確認し、その結果を公開します。
対象領域
コミットを監査しました 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
1inch/limit-order-protocol
リポジトリ。以下の契約が範囲内でした。
– OrderMixin.sol
– OrderRFQMixin.sol
– PredicateHelper.sol
– RevertReasonParser.sol
– Permitable.sol
– ChainlinkCalculator.sol
– ArgumentsDecoder.sol
– AmountCalculator.sol
– NonceManager.sol
– LimitOrderProtocol.sol
– ImmutableOwner.sol
– InteractiveNotificationReceiver.sol
– AggregatorInterface.sol
– IDaiLikePermit.sol
他のすべてのプロジェクト ファイルとディレクトリ (テストを含む)、外部依存関係とプロジェクト、ゲーム理論、インセンティブ設計もこの監査の範囲から除外されました。外部コードとコントラクトの依存関係は文書化されたとおりに機能すると想定され、1inch によって提供されるバックエンド サービスはプロトコルの利益を最大限に考慮して動作すると想定されました。
健康全般
一般的に、プロジェクトのコードベースは読みやすく、よく整理されていることがわかりましたが、特にアセンブリ コードのブロック、プロトコルのエッジ ケース、使用されるアセット/述語/外部リソースなど、より広範なドキュメントの恩恵を受ける可能性があります。提供されるバックエンド サービスの責任と制限、およびアクター間の対話。このプロジェクトでは、アクションのガス効率を高めるために多大な労力を費やしていますが、場合によっては、コードの推論がより困難になる危険性さえあります。これに関連する問題を以下に提起します。監査全体を通じて、1inch チームは可用性が高く、反応が早く、非常に協力しやすかったです。
システム概要
指値注文プロトコルにより注文が可能になります makers
トークンスワップのためにオフチェーンの注文に署名します。このプロトコルにより、以前に署名された注文を注文ごとに記入することが容易になります。 takers
。注文は拡張性が高く、注文処理プロセス全体のいくつかの時点で外部コントラクトを静的に呼び出すことができます。この拡張性により、プロトコルに実用性が与えられますが、複雑さが増し、オーダー自体の攻撃対象領域が拡大します。
注文の詳細をチェーン上に保存することはできないことに注意することが重要です。注文の約定状態またはキャンセルステータスは、注文ハッシュを介してのみ追跡されます。このため、注文をピアツーピアで、または集中化された関係者を介して共有する必要があります。この場合、1inch チームは、署名された注文を集約し、それらの注文を他のプロトコルの流動性源として使用する、集中当事者として機能する予定です。注文は独自の API を介して公開されるため、ユーザーは注文を操作できます。
この一元化により、1 インチ チームは、どの注文が発行され、最終的に実行されるかを徹底的に制御できるようになります。これにより、注文を検閲する機能も得られます。これは、悪意のある注文や欺瞞的な注文の場合に役立つ可能性がありますが、悪用されて、有利な注文の場合に API を介して表示しないことで他のユーザーを優先的に実行できる可能性もあります。
特権ロール
ロールが使用される契約は範囲外でしたが、特権ロールが 1 つ特定されました。アン immutableOwner
構築時にプロキシ契約の作成者に設定され、プロキシのアクセスを制限するために使用されます。 external
機能します。
外部の依存関係と信頼の仮定
このプロトコルの設計にはオフチェーン コンポーネントとオンチェーン コンポーネントが必要であり、このハイブリッド モデルを使用して、レポートで特定したいくつかの攻撃ベクトルを軽減できますが、その機能の代償として 1 インチ チームとインフラストラクチャへの依存度が高まります。
さらに、指値注文プロトコルは、Chainlink オラクルから価格を取得するための関数を提供します。私たちは、それらのオラクルが正直で、アクセス可能で、適切に機能するものであると仮定しました。
さらに、注文の柔軟性により、検証されていない外部契約との接点がいくつかあります。これは、悪意のあるユーザーがそのような呼び出しを悪用し、悪意のあるコントラクトを持つ述語、アセット、またはオラクルになりすまして、注文約定中にアクションを実行する可能性があることを意味します。プロジェクトは一部の領域で再入不可から保護されていますが、そのようなベクトルはサービス拒否攻撃や未検出のスパム命令を引き起こす可能性があります。 1inch チームは、プロトコルになじみのない契約を使用すると特定の問題が発生する可能性があることを認識しており、主要な「優良」資産のみがプロジェクトで完全にサポートされるという意図を示しています。ただし、最も人気のあるアセットであっても、USDT での送金中に手数料が発生したり、メッセージの代わりにエラー コードを返したりするなど、適切に対処しないプロトコル上で問題を引き起こす可能性のある各アセットの固有の動作があることに注意してください。 cToken を使用した成功のブール値。
所見
ここでは、調査結果を示します。
重大な重大度
なし。
重大度が高い
[H01] 一貫性のないデータが渡されました _makeCall
OrderMixin
契約、 _makeCall
機能は資産を転送するために使用されます 取る者から作る者へ その後 作り手から引き手へ。後者の転送では、 _makeCall
関数に注文が誤って渡されました makerAsset
最後のパラメータとして、それが注文のものである必要がある場合 makerAssetData
.
その結果、に依存するプロキシ機能はすべて、 makerAssetData
議論は破綻するだろう。
先ほどの呼び出しと一致するように、 _makeCall
また、プロキシ機能を完全にサポートするには、 order.makerAsset
パラメータを order.makerAssetData
.
アップデート: で修正されました プルリクエスト#57.
[H02] 部分的に満たされたプライベート注文は誰でも満たすことができます
このプロトコルにより、私的および公的命令の作成が可能になります。プライベートオーダーの場合のみ、 allowedSender
注文の作成時にメーカーが指定した住所で注文を完了できます。
しかし、 OrderMixin
契約する、 の検証 allowedSender
住所 スコープが間違っています。つまり、注文の最初の約定を処理するロジック内でのみ評価されます。プライベート注文が部分的に約定された場合、小切手は allowedSender
住所にアクセスできなくなり、誰でも注文を完了できるようになります。
ユーザーが部分的に約定されたプライベート注文に応じることができるべきかどうかについての意図を明確にするには、現在の動作の理由を文書化するか、 allowedSender
最初のフィルの範囲外のアドレスを使用して、フィルが試行されるたびにアドレスが検証されるようにします。
アップデート: で修正されました プルリクエスト#58.
[H03] 悪意のあるメーカーは部分フィルを利用してテイカーの資産を盗む可能性があります
からの注文 OrderMixin
契約は部分的に履行される可能性があります。部分フィルをサポートするには、プロトコルにはスワップの両側を計算する方法が必要です。両方 getMakerAmount
& getTakerAmount
フィールドは、まさにこの目的のために注文の作成者によって定義されます。
注文に応じる際、受付者は次のいずれかを提供する必要があります。 makingAmount
または takingAmount
価値観だけでなく、 thresholdAmount
価値。使用できるコードパスは 2 つあります。 makingAmount
または takingAmount
が提供されました。
1つ目は、 makingAmount
パラメータが定義されています。出来た 切り詰める makingAmount
価値とまた 計算する takingAmount
それに対する価値。この状況では、 thresholdAmount
確実に takingAmount
取得される値は 意外と大きくない.
2つ目は、 takingAmount
パラメータが定義されています。そのような場合には、 計算する makingAmount
値、可能性あり それを切り捨てる & 再計算する takingAmount
それが起こった場合の値。この状況では、 thresholdAmount
値により、 makingAmount
返される値は 意外と小さくない.
2 つの悪用方法が存在し、それぞれ前述のコードパスの 1 つに固有です。これらの悪用方法には悪意のあるものが必要です。 getMakerAmount
& getTakerAmount
機能。これらの関数を単純に実装すると、次と同じ動作になります。 AmountCalculator
〜の getMakerAmount
& getTakerAmount
ただし、必要に応じて攻撃者が制御する値を強制的に返すハードコードされたスイッチを備えています。
1 つ目の、それほど深刻ではない悪用パターンには、最初のコードパスが含まれます。 makingAmount
値が指定されている 約定注文で。悪意のあるメーカーは、以下を指定する約定注文を待ちます。 makingAmount
先頭に立つためにメンプールに現れる。彼らは作成者側から 1 を除くすべての値を排出してから強制します。 _callGetTakerAmount
ユーザーに指定された金額を返す thresholdAmount
値(またはそれより低い場合は許容値)。ユーザーのトランザクションが最終的に完了すると、ユーザーは完全にスワップします。 thresholdAmount
値します takerAsset
1単位あたり makerAsset
。このエクスプロイトは、 thresholdAmount
の価値または量 takerAsset
で許可されたユーザー LimitOrderProtocol
契約する。
2 番目のより深刻な悪用パターンには、2 番目のコードパスが含まれます。 takingAmount
値が指定されている。悪意のあるメーカーも同様に、 takingAmount
メモリプールに表示される値。彼らはトランザクションを前倒しで実行し、 makingAmount
によって返される値 _callGetMakerAmount
関数は両方よりも高くなります remainingMakerAmount
と thresholdAmount
。彼らはまた、 takingAmount
戻り値 _callGetTakerAmount
の量になる takerAsset
で許可されている資産 LimitOrderProtocol
テイカーによって。テイカーのトランザクションが完了すると、 を切り詰める makingAmount
値を設定してから を再計算します takingAmount
価値。ただし、この再計算はより低いという保証はなく、この場合、テイカーはすべてのデータを使い果たします。 takerAsset
彼らは契約上それを許可しました。このコードパスでは、 thresholdAmount
値は 確実に makingAmount
低すぎませんなので、すべてのテイクを受け取ります takerAsset
アセットはチェックされていません。失われた資金は次の金額に制限されます。 takerAsset
ユーザーが許可した資産 LimitOrderProtocol
契約する。
これらのエクスプロイトは、部分的な注文、より具体的には、悪意のある部分的な注文がなければ不可能です。 getMakerAmount
& getTakerAmount
実装
の主な問題は、 thresholdAmount
値チェックは、スワップの片側のみをカバーしますが、もう一方の側はフロントランニングによって操作できることです。テイカーが最初に提案した値が変更されないという保証はありません。削除を検討してください makingAmount
両方のコードパスから切り捨てられ、注文が要求されたほどの大きさの約定をサポートできない場合は元に戻ります。こうすることで、 thresholdAmount
を使用すると、スワップの相手側を十分に制限し、悪意のある注文であっても予期しない動作を回避できます。
アップデート: で修正されました プルリクエスト#83.
中程度の重大度
[M01] 動的引数の後に渡される静的引数
OrderMixin
契約、 getTakerAmount
& getMakerAmount
bytes フィールドは、 _callGetTakerAmount
& _callGetMakerAmount
機能。これらの呼び出しは、スワップの一方の側をもう一方の側に基づいて計算する方法を提供し、ユーザーが部分的に注文を実行できるようにします。
getTakerAmount
/getMakerAmount
フィールドは動的変数であり、フィールドの前にパックされます。 takerAmount
& makerAmount
の値 _callGetTakerAmount
& _callGetMakerAmount
機能。悪意のあるメーカーが予想を超えるデータを提供する可能性があります。 getTakerAmount
&getMakerAmount
プッシュするフィールド takerAmount
& makerAmount
次の関数でデコードされるときに想定される場所を超えたバイト数。これにより、メーカーは、渡されたテイカーまたはメーカーの量を右に全バイトシフトすることができ、追加の 32 バイトのデータが提供された場合はそれらを完全に置き換えることもできます。
ユーザーはすでに手動でレビューする必要があります getTakerAmount
& getMakerAmount
フィールドは順番に並べられていますが、この手法を見つけるのはかなり困難です。また、この攻撃は内部的に信頼されているユーザーにも適用されることにも注目してください。 getMakerAmount
& getTakerAmount
機能。ほとんどの攻撃では、適切なしきい値を設定することで資金の損失を防ぐことができます。
これを防ぐには、静的引数を制御するメソッドが動的引数に与えられないように、動的引数の前に静的引数をエンコードすることを検討してください。
アップデート: 未修理。 1インチチームは次のように述べています。
ゲッターの検証には細心の注意を払います。潜在的に悪意のある注文をフィルタリングするのに役立つゲッターの健全性検証を SDK に実装しようとします。
[M02] ERC721注文を操作可能
ERC20 以外のものを交換することも可能です。 OrderMixin
IERC20 と同じ関数セレクターを共有するコントラクトを展開することによって transferFrom
、そしてその契約を makerAsset
または takerAsset
注文で。
範囲外のプロキシ、つまり、 ERC721Proxy
, ERC721ProxySafe
, ERC1155Proxy
契約はこのパターンに従ってサポートを提供します ERC721
& ERC1155
トークン。プロキシは IERC20 と同じパターンで呼び出される必要があるため、 transferFrom
呼び出しの場合、署名は次で始まる必要があります address from
, address to
& uint256 amount
。プロキシが必要とするその他のものはすべて後に渡すことができ、順序で次のように定義されます。 makerAssetData
& takerAssetData
.
ERC1155 は、当然のことながら、複数の同じ ID トークンを一度に転送できます。 ERC1155Proxy
契約では、 amount
分野。一方で、 ERC721
明確な用途はありません amount
分野。これらは代替不可能なトークンを表すため、特定の tokenId には 1 つだけが存在し、 amount
フィールドは役に立たない。このため、両方の実装は ERC721Proxy
& ERC721ProxySafe
契約では必要なものを使用します amount
フィールドとして tokenId
を代わりにお使いください。
この過負荷は、 amount
パラメータにより部分的に埋められる可能性が生じます ERC721
別途リストされているトークンを割引価格で購入するための注文。たとえば、1 人のユーザーが複数のユーザーを所有している場合があります。 ERC721
同じ契約の譲渡が許可されているもの ERC721Proxy
契約し、それらを個別の指値注文にリストします。
指値注文でも提供される場合は、 getMakerAmount
& getTakerAmount
フィールドに部分的に入力することも可能です ERC721
命令。注文以来 amount
フィールドは実際には tokenId
、悪意のあるユーザーが部分的な塗りつぶしを配置する可能性があります。 ERC721
tokenId が高くなると、 makingAmount
/takingAmount
ERC721
それはより低いものに対応する可能性があります tokenId
。 結果は ERC721
低いと tokenId
の価格で譲渡されることになる (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
このエクスプロイトにはいくつかの要件があります。
- 複数
ERC721
同じ契約に基づくものはどちらでも許可されますERC721
単一の所有者による代理。 - いずれかのオープンオーダー
ERC721
それは最低ではないtokenId
許可されているもののうち。 - 注文では部分的な約定が許可されます。
部分的な可能性を完全に排除するには ERC721
塗りつぶしの場合は、分割することを検討してください。 amount
& tokenId
引数。引数が分離されているかどうかに関係なく、ユーザーにこの動作について警告し、今後このパターンを避けるために、これを文書化することも検討してください。
アップデート: で修正されました プルリクエスト#59.
[M03] 文書化されていない XNUMX 進数の仮定
LimitOrderProtocol
契約は継承します ChainlinkCalculator
を通じて契約する OrderMixin
契約。このコントラクトは、Chainlink オラクルの使用を可能にする 2 つの機能を公開します。 述語チェック そして、の検索 メーカー金額/引受金額.
ただし、この契約では、Chainlink オラクルがレポートする必要がある小数点以下の桁数と、関数パラメーターに含める必要がある小数点以下の桁数について、文書化されていない仮定を行っています。特定のシナリオでは、これにより、資産の価格設定の誤りや意図しない資金の損失など、予期しない動作が発生する可能性があります。
より具体的には、契約全体を通じて、Chainlink のオラクルは小数点以下 18 桁の精度でレポートすることが暗黙の前提となっています。ただし、そうではありません すべてのチェーンリンクの神託 この小数点以下の桁数でレポートします。実際、オラクルが通貨 (USD など) に基づいてトークン ペアを報告する場合、その精度は小数点以下 8 桁のみです。制限はありませんので、 which オラクルを使用することはできますが、レポートに使用される小数点以下の桁数について暗黙の想定を行うべきではありません。
これに関連して、次のような暗黙の仮定があります。 amount
以下のためのパラメータ ChainlinkCalculator
関数は、誤解を招く明示的な宣言とともに、18 桁の XNUMX 進数を使用します。 singlePrice
function Calculates price of token relative to ETH scaled by 1e18
。実際には、そのような神託があっても、 ありません 小数点以下 18 桁のレポート、戻り値 singlePrice
関数は、小数点以下の桁数によってスケーリングされます。 amount
パラメータは、必ずしも小数点以下 18 桁であるとは限りません。
同様に、 doublePrice
関数は、2 つの Chainlink オラクルが同じ小数点以下の桁数でレポートすることを前提としているため、関数の結果が期待から逸脱します。
パラメーターと戻り値の小数点以下の桁数に関する仮定を明示的に文書化することを検討してください。さらに、これらの仮定を破るオラクルに依存する計算を制限するか、関連する計算で実際の小数点以下の桁数を考慮することを検討してください。
アップデート: で修正されました プルリクエスト#75.
重大度が低い
[L01] 明示的に宣言されていない定数
コードベース内で説明のない意味でリテラル値が使用されることがいくつかあります。例えば:
-
OrderMixin
契約、_remaining
マッピングは意味的にオーバーロードされています (問題で説明されているように) マッピングのセマンティックなオーバーロード) 部分的に約定された注文の残りの資産額を追跡するため と同様 注文が完全に満たされた場合。具体的には、0
注文に関連付けられた約定が行われていないことを意味します。1
これ以上注文に応じることができないことを意味し、それより大きいものは1
これは、注文に関連付けられた残高があり、約定できる可能性があることを意味します。 -
ChainlinkCalculator
契約、文字通りの値1e18
で使用されていますsinglePrice
機能。
コードの読みやすさを向上させ、リファクタリングを容易にするために、すべてのマジックナンバーに定数を定義し、明確でわかりやすい名前を付けることを検討してください。複雑な値の場合は、値がどのように計算されたか、またはその値が選択された理由を説明するインライン コメントを追加することを検討してください。
アップデート: で修正されました プルリクエスト#75 & プルリクエスト#76.
[L02] 悪意のある当事者により、許可された命令の実行が妨げられる可能性があります
OrderMixin
メーカーユーザーが提出できる契約 許可される命令 そのため、承認のために別のトランザクションを行う必要がなく、1 つのトランザクションでこれらを実行できます。また、注文担当者は、 独自の許可を提出する 同じ目的での注文の実行中に。
ただし、メーカーの許可が入っているため、 注文、オーダーフィルトランザクションがメモリプール内にある間、メーカーの許可とテイカーの許可の両方にアクセスできます。これにより、悪意のあるユーザーがこれらの許可を取得し、フィル トランザクションをフロントランニングしながら、それぞれの資産コントラクトに対して実行することが可能になります。なぜなら、これらの許可には nonce
二重支払い攻撃を防ぐために、フロントラン中に使用されたのと同じ許可を使用しようとした結果、注文の約定トランザクションは失敗します。
セキュリティ上のリスクはなく、メーカーは新しい注文を作成してトランザクションを事前承認することができますが、この攻撃は、許可されている注文の使いやすさに確実に影響を与える可能性があります。実際、動機のある攻撃者はブロックする可能性があります。 を この攻撃で許可される命令。注文が満たされるときに、許可がすでに提出されているかどうか、または許容量が十分であるかどうかを検証することを検討してください。また、注文の作成中にこの攻撃の可能性についてユーザーに知らせることも検討してください。
アップデート: 未修理。 1インチチームは次のように述べています。
以前にも承認チェックを行っていましたが、不合格の承認のみを取り消すために許可フローを簡素化することにしました。メーカーへの通知方法を検討してまいります。
[L03] コードが重複しています
コードベース内に重複したコードのインスタンスがあります。コードを複製すると、開発ライフサイクルの後半で問題が発生する可能性があり、プロジェクトにエラーが発生しやすくなります。このようなエラーは、機能の変更が同一であるべきコードのすべてのインスタンスにわたって複製されていない場合に、誤って発生する可能性があります。重複したコードの例は次のとおりです。
コードを複製するのではなく、複製されたコードを含むコントラクトまたはライブラリを 1 つだけ用意し、複製された機能が必要なときはいつでもそれを使用することを検討してください。
アップデート: 部分的に修正されました プルリクエスト#60.
[L04] 誤ったまたは誤解を招くテストスイート
テスト スイートには、テストが予想される動作から逸脱するインスタンスがあります。例えば:
-
ChainlinkCalculator
契約は継承されますOrderMixin
契約。ただし、テスト中に、AmountCalculator.arbitraryStaticCall
関数は、ChainlinkCalculator
外部の独立した契約としての契約。結果が予期されたものであっても、テストは、呼び出しによってシステムの現在の設計と予期されるユースケースでの動作を反映する必要があります。ChainlinkCalculator
任意の静的呼び出しを使用せずに直接関数を呼び出します。 - プロキシ契約は範囲外でしたが、ERC721 アセットを使用してプロトコルをテストしたときに、
ERC721Proxy
契約は資産の交換には使用されません。 テストスイート.
テスト スイート自体はこの監査の範囲外であるため、テスト スイートを徹底的にレビューして、すべてのテストがプロトコルの仕様に従って正常に実行されることを確認することを検討してください。
アップデート: で修正されました プルリクエスト#57, プルリクエスト#59, プルリクエスト#61.
[L05] イベントの誤りと脱落
通常、コードベース全体で、コントラクトに機密の変更が加えられたときにイベントが発行されます。ただし、多くのイベントにはインデックス付きパラメータが不足していたり、重要なパラメータが欠落していたりします。例えば:
次のような、イベントが欠落している機密性の高いアクションもあります。
既存のイベントのインデックスをより完全に作成し、不足している新しいパラメータを追加することを検討してください。また、オフチェーン サービスによるコントラクトの状態の再構築に使用できるように、すべてのイベントを完全な方法で発行することを検討してください。
アップデート: 未修理。ただし、1 インチ チームは次の機能を追加しました。 orderRemaining
へのパラメータ OrderCanceled
イベント中 プルリクエスト#62.
1インチチームは次のように述べています。
フロントエンドのニーズを満たすには、限られたデータのサブセットのみが必要であることがわかりました。広範な分析の場合、推奨されるフィールドはすべてトレースを通じて利用できます。のために
OrderRFQMixin
私たちは、マーケットメーカーがどの注文がキャンセルされたかを追跡するための独自の洗練された方法を構築することを期待しています。
[L06] イベント発生時のストレージ変更
NonceManager
契約時、 NonceIncreased
イベントが発行され、 メッセージ送信者のナンスも増加します.
複数の操作を同時に実行すると、コードベースの推論が難しくなり、エラーが発生しやすくなり、操作の見落としや誤解につながる可能性があります。
コード全体の意図性、読みやすさ、明瞭さを向上させるには、イベントを発行する前に nonce 値を増やすことを検討してください。
アップデート: で修正されました プルリクエスト#63.
[L07] 一貫性のないデコード方法により、結果の不一致が発生する可能性があります
拡張性と柔軟性をすべてサポートするために、指値注文プロトコルは動的バイト データと外部コントラクトからの任意の戻り値を日常的に処理する必要があります。その結果、プロトコルには次のものが含まれます。 ArgumentsDecoder
ライブラリを使用して、動的バイト値を基本データ型に効率的に変換します。ただし、このライブラリは排他的に使用されるわけではなく、場合によっては abi.decode
代わりに使用されます。さらに、一部の契約では、 abi coder v1
他の人が使っている間 abi coder v2
。前者のパフォーマンスは、 ArgumentsDecoder
一方、後者はデコード時に追加のチェックを実行します。
これらの異なるデコード方法の一貫性のない使用により、コードベースの意図と実際の動作の間に微妙な矛盾が生じる可能性があります。
例えば、 simulateCalls
関数のみを使用します ArgumentsDecoder.decodeBool
関数。 の場合 simulateCalls
この関数は、注文の述語部分で行われる呼び出しをチェックするために使用されますが、異なるデコード手法が使用されるため、その結果は述語条件が評価されるときに実際に発生するものと異なる可能性があります。
したがって、たとえば、述語が外部を作成する場合、 staticcall
を返す何らかの関数に対して、 uint256
予想よりも 1 より大きい値 bool
の場合、戻り値は次のとおりであるため、その呼び出しは元に戻ります。 でデコードされた abi coder v2
〜の abi.decode
次のような値は受け入れられません。 bool
。ただし、まったく同じ呼び出しが行われた場合、 simulateCalls
、 それから 単に次のようにマークされます true
、理由 decodeBool
ゼロより大きい値はすべて次のように扱われます。 true
.
するために simulateCalls
関数は実際の述語呼び出しの動作を完全にミラーリングします。使用するように変更することを検討してください。 abi.decode
.
アップデート: で修正されました プルリクエスト#82.
[L08]入力検証の欠如
fillOrderToWithPermit
& fillOrderTo
の機能 OrderMixin
契約書だけでなく、 fillOrderRFQToWithPermit
& fillOrderRFQTo
の機能 OrderRFQMixin
契約書を検証しないでください target
アドレスパラメータ。
これにより、ユーザーが誤ってゼロアドレスを渡してしまい、その結果、注文完了後に受け取るはずだった資産がロックされてしまう可能性があります。
ユーザーが誤って資金をロックしないようにするには、次のことを検証することを検討してください。 target
address は、引用された関数のゼロ アドレスと等しくありません。
アップデート: で修正されました プルリクエスト#78.
[L09] 単体テストのカバレッジが低い
プロジェクト全体の単体テストのカバー率は約 75% ですが、一部の契約ではカバー率が特に低くなります。
リファクタリングや新機能の開発時にコードを検証しリグレッションを防ぐための単体テストの重要性を考慮し、単体テストのカバレッジを少なくとも 95% まで大幅に高め、ありそうもない状況もカバーするエッジ ケースを含めることをお勧めします。
アップデート: 未修理。
[L10]誤解を招くまたは不完全なインラインドキュメント
コードベース全体で、誤解を招く、または不完全なインラインドキュメントのいくつかのインスタンスが特定されたため、修正する必要があります。
以下は、誤解を招くインラインドキュメントの例です。
-
ChainlinkCalculator
契約、singlePrice
機能 ナットスペック@notice
タグ それは言うCalculates price of token relative to ETH scaled by 1e18
、しかし実際には、その結果は、 値 ofamount
トークンのスケーリング1e18
、ここで、オラクルはETHに関して報告しない場合があります(たとえば、ETHを含まないペアの場合)。 -
OrderRFQMixin
契約、invalidatorForOrderRFQ
機能 ナットスペック@return
タグ これは、それぞれの無効化ビットが設定されるように引用符が記入されていない可能性があるため、誤解を招きます。注文がキャンセルされた可能性もあります。 - 回線上 147, 165, 188 of
OrderMixin.sol
、NatSpec@param
タグは文法的ではありません。 - オンライン 20 of
ERC1155Proxy.sol
@notice
タグは、計算されたハッシュが、func_733NCGU
関数、あるべき場所func_301JL5R
代わりに機能します。
以下は、不完全なインラインドキュメントの例です。
- の機能
AmountCalculator
契約にはパラメータがまったく記載されていません。 -
ChainlinkCalculator
契約、singlePrice
&doublePrice
関数はすべてのパラメータを記述しているわけではありません。 -
ImmutableOwner
契約では、パブリック変数と修飾子には NatSpec がありません。 -
InteractiveNotificationReceiver
契約、notifyFillOrder
関数にはパラメータがまったく記述されていません。 -
LimitOrderProtocol
契約、DOMAIN_SEPARATOR
この関数には NatSpec がありません。 - のイベントとマッピング
NonceManager
NatSpec がありません。 -
OrderRFQMixin
契約する、cancelOrderRFQ*
関数は戻り値を記述しません。 -
OrderMixin
契約上、いくつかの関数には完全な NatSpec がありません。 - オンライン 168 of
OrderMixin.sol
そしてオンラインで 71 ofOrderRFQMixin.sol
が欠けています。@dev
タグ。 - の機能
PredicateHelper
契約書にはすべてのパラメータが記載されているわけではありません。
明確なインライン ドキュメントは、コードの意図を概要説明するための基本です。インラインドキュメントと実装の間に不一致があると、システムがどのように動作すると予想されるかについて重大な誤解が生じる可能性があります。開発者、ユーザー、監査人などの混乱を避けるために、これらのエラーを修正することを検討してください。
アップデート: 部分的に修正されました。誤解を招く文書については、 プルリクエスト#75 & プルリクエスト#77.
1インチチームは次のように述べています。
誤解を招くドキュメントを修正しました。ドキュメントの完成は後で行われます。
[L11] フック使用時に DoS 命令が可能
OrderMixin
コントラクトは、成功の条件を持つ可能性のある一般的なオフチェーン スワップ注文を満たす機能を実装します。注文約定中に、注文は 事前定義された「述語」条件を確認する 実行を続行する前に。
ただし、これらの述語条件は任意のコントラクトのロジックをターゲットにする可能性があるため、悪意のある作成者は注文が正しく動作し、オフチェーンでチェックする際には有効であるとテイカーを騙し、同じ注文を約定しようとすると失敗する可能性があります。オンチェーン。述語の動作のこの変更は、述語が依存する何らかの変数状態をフロントランニングすることによって、送信されたガスや呼び出しに含まれるアドレスを調べることによって、またはその他のロジックによって行うことができます。
さらに、メーカーが定義した場合、 交換中のやり取り interactionTarget
契約の内容が元に戻ったり、注文の成立を妨げるために許可が取り消されたりする可能性があり、本質的には悪意のある述語と同じ結果を招く可能性があります。
資産が危険にさらされることはありませんが、有利な注文を見つけたユーザーやボットは、表面的には正当に見えるこの種のスパム注文を特定する負担が増大します。この種の注文を特定できなかった場合、無駄なガス代が発生することになります。スパム注文の量を減らすには、これらのフックで使用可能なターゲットを制限することを検討してください。また、注文に応じようとする前に、この可能性についてユーザーに警告することも検討してください。
アップデート: 未修理。 1インチチームは次のように述べています。
私たちはそれをバックエンドで処理し、潜在的な参加者に問題について通知する方法を検討します。
[L12] 丸めは不利になる可能性があります taker
OrderMixin
& OrderRFQMixin
注文が約定され、テイカーが注文のみを提供した場合の契約。 makingAmount
or takingAmount
金額を指定すると、プロトコルはスワップの相当額を計算しようとします。
これらの計算には 2 つの問題があります。1 つ目は、金額パラメータで使用する小数点以下の桁数を制限するドキュメントやロジックがないことです。これについては、 文書化されていない 10 進数の仮定 問題。
2 番目の問題は、これらの計算の過程で、プロトコルが作成者に有利になるように丸められることです。丸めの問題は、暗黙の小数の前提が崩れると大幅に悪化する可能性がありますが、すべてが期待どおりの場合でも、小さく奇数の金額で丸めが発生します。
受取人が最低金額を指定できるようにすることを検討してください。 makerAsset
最大額の資産と一緒に受け取りたい資産 takerAsset
これにより、端数処理の受け入れがより明確になります。
アップデート: 未修理。 1インチチームは次のように述べています。
しきい値はテイカーを保護するのに十分な量である必要があります。
[L13] パラメータ不足時の矛盾した注文の処理
OrderMixin
契約、 fillOrderTo
関数は内部呼び出しを行います _callGetMakerAmount
& _callGetTakerAmount
フィルが試行されるたびに機能します。 makingAmount
または takingAmount
パラメータがそれぞれゼロであるか、 makingAmount
値が remainingMakerAmount
の値です。
_callGetMakerAmount
& _callGetTakerAmount
注文が getMakerAmount
or getTakerAmount
それぞれパラメータが設定され、部分的なフィルが実行されます。
An インラインコメントの横に _callGetMakerAmount
と インラインコメントの横に _callGetTakerAmount
注文が作成されたものでない場合、「全約定のみが許可される」と主張する getMakerAmount
or getTakerAmount
パラメーター。
ただし、これらのパスではチェックが行われないため、これが適用されないコード パスがあります。 length
両方の getMakerAmount
& getTakerAmount
パラメーター。
具体的には、 時 taker
を指定します takerAmount
のみを持つ注文の値 getMakerAmount
、その呼び出しがない限り、 getMakerAmount
より大きい金額を返します remainingMakerAmount
、インライン ドキュメントに反して部分的な塗りつぶしが実行される可能性があります。
このため、これらのコード パスの意図性が不明瞭になります。これが予想される動作である場合は、より明示的にインライン ドキュメントを変更することを検討してください。これが意図的でない動作である場合は、両方の長さを常に確認することを検討してください。 getMakerAmount
と getTakerAmount
これにより、インライン ドキュメントで説明されている動作が実装で強化されます。
アップデート: で修正されました プルリクエスト#79.
[L14] 非推奨の Chainlink 呼び出しの使用
ChainlinkCalculator
コントラクトは、Chainlink オラクルをクエリするために使用されることを目的としています。これは、相手に電話をかけることで行われます。 latestTimestamp
& latestAnswer
メソッド どちらも非推奨になりました。実際、このメソッドは Chainlink アグリゲーターの API には存在しません。 バージョン 3 以降.
Chainlink オラクルとの将来の潜在的な非互換性を回避するには、 latestRoundData
代わりにメソッド。
アップデート: で修正されました プルリクエスト#67.
メモと追加情報
[N01] インターフェイスをインポートしていません
AggregatorInterface
インターフェイスは、からコピーされたコードのサブセットであるようです ChainLink
のパブリックコードリポジトリ。完全なインターフェースは以下に含まれています ChainLink
の契約 npm パッケージ.
可能であれば、インターフェイスの不一致やその結果として生じる問題の可能性を減らすために、別のプロジェクトのインターフェイスを再定義したり書き直したりするのではなく、代わりに公式の npm パッケージ経由でインストールされたインターフェイスを使用することを検討してください。
アップデート: で修正されました プルリクエスト#66.
[N02]非推奨のプロジェクト依存関係
インストール中 プロジェクトの依存関係、NPMは、パッケージのXNUMXつがインストールされていることを警告します。 Highlight
、「サポートされなくなるか、将来的にセキュリティアップデートを受信します」。
このパッケージがセキュリティリスクを引き起こす可能性は低いですが、このパッケージを使用する依存関係を維持されたバージョンにアップグレードすることを検討してください。
アップデート: で修正されました プルリクエスト#64.
[N03] ビューメソッドへの外部呼び出しは静的呼び出しではありません
コードベースの大部分を通じて、このプロトコルは OpenZeppelin を介して外部呼び出しを明示的に行います。 functionStaticCall
状態変化が予期されない場合、または望ましくない場合に、状態変化の可能性を制限する方法。ただし、 ChainlinkCalculator
外部呼び出しのみを目的としているにもかかわらず、契約 view
Chainlink オラクルのメソッド、 singlePrice
& doublePrice
関数は明示的に作成されません staticcall
s.
これに起因する差し迫ったセキュリティ上の懸念は確認されませんでしたが、攻撃対象領域を減らし、一貫性を向上させ、意図を明確にするために、明示的なセキュリティの使用を検討してください。 staticcall
s、すべての外部呼び出しに対して view
の機能 ChainlinkCalculator
契約する。
アップデート: 未修理。 1インチチームは次のように述べています。
私たちは、構文が複雑になると一貫性の向上が無効になると考えています。
[N04] 無効な注文でも早期に失敗しない
OrderMixin
契約、 fillOrderTo
この関数は、注文がまだ送信されていない場合の特別な条件を処理します (remainingMakerAmount == 0
) ですが、注文が有効でなくなったときの条件は明示的に処理されません (remainingMakerAmount == 1
).
後者のシナリオでは、機能は最終的に元に戻りますが、それは無視できない量のガスが燃焼した後でのみです。意図を明確にし、可読性を高め、ガスの使用量を削減するには、関数の先頭で無効な順序のシナリオを明示的に処理することを検討してください。
アップデート: で修正されました プルリクエスト#68.
[N05] ヘルパー契約が抽象としてマークされていない
Solidityのキーワードは、 abstract
は、それ自体が機能する契約ではない、またはそのように使用されることを意図していない契約に使用されます。その代わり、 abstract
コントラクトはシステム内の他のコントラクトに継承され、スタンドアロンの機能コントラクトが作成されます。
コードベース全体には、単独でデプロイすることを意図していないにもかかわらず、抽象としてマークされていないヘルパー コントラクトの例があります。たとえば、 AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
, PredicateHelper
コントラクトはすべて、継承するコントラクトによって使用されることを目的とした関数の基本セットで構成されています。
ヘルパー契約を次のようにマークすることを検討してください。 abstract
それらを継承するコントラクトに機能を追加することのみを目的として設計されていることを明確に示します。
アップデート: 未修理。 1インチチームは次のように述べています。
これらのヘルパーは個別にデプロイできます。それらはガスの節約のためだけに継承されます。
[N06] 一貫性のない関数の順序付け
コードベース全体を通じて、関数の順序付けは一般に次のとおりです。 Solidity スタイル ガイドで推奨される順序、これは次のようになります。 constructor
, fallback
, external
, public
, internal
, private
.
ただし、 OrderMixin
契約、 public
checkPredicate
関数はスタイルガイドから逸脱し、 external
機能します。
プロジェクト全体の読みやすさを向上させるには、Solidity スタイル ガイドで推奨されているように、コードベース全体で関数の順序を標準化することを検討してください。
アップデート: で修正されました プルリクエスト#69.
[N07] 一貫性のない注文約定フロー
OrderMixin
& RFQOrderMixin
どちらの契約も署名済みの注文の履行を処理しますが、2 つの契約間の一般的な注文フローには一貫性がありません。
OrderMixin
〜の fillOrderTo
関数は次の一般的なフローに従います (疑似コード)。
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
それに対して RFQOrderMixin
の類似物 fillOrderRFQTo
関数は次のフローに従います (疑似コード)。
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
これら 2 つの関数それぞれの最初の条件がなぜ異なるのか、あるいはなぜ異なるのかについては、ドキュメントからは洞察が得られません。 takingAmount
& makingAmount
後者の関数では両方をゼロにすることはできません。また、両方とも makingAmount
フォルダーとその下に takingAmount
が提供されているため、推論するのがはるかに簡単です fillOrderRFQTo
関数は最終的に明確に処理されるため、 else
ブロック。
意図を明確にし、コード全体の可読性を高めるには、これら 2 つの契約にわたる一般的な注文フローを標準化するか、違いが存在する理由を明示的に文書化することを検討してください。
アップデート: 未修理。 1インチチームは次のように述べています。
これは、指値注文のカスタム価格設定機能によるものです。以来
getMakerAmount
~と大幅に異なる可能性がありますgetTakerAmount
、それらのゲッターが異なる場合にテイカーを混乱させる可能性があるため、テイカーのデフォルト オプションを作成しない方がよいと考えました。
[N08] エラー メッセージの形式が一貫していない、または誤解を招く
コードベース全体を通して、 require
& revert
トランザクションの失敗を引き起こす特定の状況をユーザーに通知するためのエラー メッセージの形式が一貫していないことが判明しました。
たとえば、行上の各エラー メッセージは、 の85 OrderMixin.sol
, の16 ERC721ProxySafe.sol
, の26 Permitable.sol
異なるスタイルを採用します。
さらに、いくつかのエラー メッセージは誤解を招きます。
エラー メッセージは、障害状態についてユーザーに通知することを目的としているため、システムと対話するために適切な修正を行うことができるように、十分な情報を提供する必要があります。有益ではないエラー メッセージは、ユーザー エクスペリエンス全体に大きなダメージを与え、システムの品質を低下させます。さらに、エラー メッセージの形式が一貫していない場合、不要な混乱が生じる可能性があります。したがって、コードベース全体をレビューして、すべての点が確実に行われていることを確認することを検討してください。 require
& revert
ステートメントには、一貫した形式で、正確で、有益で、ユーザーフレンドリーなエラー メッセージが含まれています。
アップデート: 部分的に修正されました プルリクエスト#81.
[N09]名前付き戻り変数の一貫性のない使用
名前付き戻り変数の使用に一貫性がありません OrderMixin
契約。一部の機能 名前付き変数を返す、 その他 明示的な値を返す、 その他 名前付き戻り変数を宣言しますが、それをオーバーライドします 明示的な return ステートメントを使用します。
すべての名前付き戻り変数を削除し、それらをローカル変数として明示的に宣言し、必要に応じて必要なreturnステートメントを追加することにより、コードベース全体で値を返すための一貫したアプローチを採用することを検討してください。 これにより、コードの明示性と可読性の両方が向上し、将来のコードリファクタリング中のリグレッションを減らすのにも役立つ可能性があります。
アップデート: で修正されました プルリクエスト#73.
[N10] 注文のハッシュ計算は API に対して公開されていません
external
機能 remaining
, remainingRaw
& remainingsRaw
すべては、操作が成功した場合の注文ハッシュを期待します。
ただし、ヘルパー関数は、 _hash
注文のハッシュを返す、 private
視認性。つまり、注文のハッシュを取得するには、ユーザーが注文の一部とドメイン文字列を手動でパックする必要があります。
注文ハッシュを計算する際の間違いの可能性を回避し、注文のそれぞれのハッシュを生成する方法をユーザーに提供するには、 _hash
機能する public
名前をリファクタリングすると、 hash
コードの残りの部分と一貫性を持たせるためです。
アップデート: で修正されました プルリクエスト#74.
[N11] マッピングのセマンティック オーバーロード
_remaining
でのマッピング OrderMixin
契約は、注文のステータスとそれらの注文に利用可能な資産の残りの量を追跡するために意味的にオーバーロードされます。
取り得る 3 つの状態は次のとおりです。
0
: 注文ハッシュはまだ確認されていません。1
: 注文はキャンセルされたか、完全に注文されました。- 任意
uint
より大きい1
: 残りmakerAmount
オーダープラスで記入可能1
.
このセマンティック オーバーロードでは、この値のラップとラップ解除が必要になります。 lookup
, cancellation
, initialization
, storage
.
セマンティック オーバーロードとそれを有効にするために必要なロジックはエラーを起こしやすく、コードベースの理解や推論を難しくする可能性があります。また、将来のコード更新で回帰の扉が開く可能性もあります。
コードの可読性を向上させるには、別のマッピングで注文の完了状態を追跡することを検討してください。
アップデート: 未修理。 1インチチームは、修正によりガスコストが増加する可能性があると述べました。 fillOrder
機能。
[N12] 許可付きの注文では任意の契約への呼び出しが可能
OrderMixin
契約は継承します Permitable
そのようなものを受け入れる資産で単一トランザクションの注文を満たすことを可能にする契約 permit
手当の変更を求める電話。
しかし、 に電話をかける Permitable
縮小することはできません。 ターゲットが許可される資産であるかどうか、さらには資産であるかどうかを検証しないと、悪意のあるユーザーが任意のコントラクトのアドレスを渡して、注文約定が完了する前に別の呼び出しを実行する可能性があります。
契約書はあるものの、 再入可能から保護される、攻撃対象領域を減らし、実行中の外部コントラクトの呼び出しを防止することが常に推奨されます。許可に関係する資産を注文に関係する資産に制限するか、プロトコルの資産ホワイトリストに制限することを検討してください。
アップデート: 未修理。 1インチチームは次のように述べています。
OrderMixin
実際のトークンに関する情報は実際にはありませんmakerAsset
&takerAsset
場合によってはプロキシまたは他の中間コントラクトであり、実際のトークンに関する情報は任意のバイトに保存されます。したがって、どの資産を制限する有効な方法はありませんpermit
が呼び出されます。
【N13】 solhint
再び有効になることはありません
コードベース全体を通して、いくつかの点があります。 solhint-disable
発言、特にオンラインでの発言 23 そしてオンラインで 41 of RevertReasonParser.sol
で終了していないもの solhint-enable
.
明示性を優先し、無効にする場合は可能な限り制限的にする solhint
、使用を検討してください solhint-disable-line
or solhint-disable-next-line
代わりに、行と同様に 16 同じファイルの。
アップデート: で修正されました プルリクエスト#72.
[N14]タイプミス
コードベースには、次のタイプミスが含まれています。
さらにプロジェクトの README
(この監査の範囲外) には次のタイプミスが含まれています。
コードの可読性を向上させるために、これらのタイプミスを修正することを検討してください。
アップデート: で修正されました プルリクエスト#71 & プルリクエスト#77.
[N15] の使用 uint
uint256
明示性を優先するために、 uint
として宣言する必要があります uint256
。特に、 for
行上のループ 98 & 119 of OrderMixin.sol
と行 16 & 30 of PredicateHelper.sol
.
アップデート: で修正されました プルリクエスト#70.
結論
重大度の高い問題が 3 件見つかりました。ベスト プラクティスに従い、潜在的な攻撃対象領域を減らすために、いくつかの変更が提案されました。
- &
- 7
- 私たちについて
- アクセス
- 従った
- 越えて
- 行為
- 行動
- NEW
- 住所
- 利点
- すべて
- 許可
- 既に
- しかし
- 金額
- 分析
- API
- アプローチ
- 引数
- 周りに
- 資産
- 資産
- 監査
- バックエンド
- 開始
- さ
- BEST
- ベストプラクティス
- ビット
- ボット
- ビルド
- コール
- これ
- 例
- 原因となる
- チェーンリンク
- 変化する
- 点検
- 小切手
- コード
- 複雑な
- 条件
- 混乱
- 建設
- 含まれています
- 縮小することはできません。
- 契約
- 補正
- コスト
- 可能性
- カップル
- クリエイター
- 通貨
- 電流プローブ
- データ
- 取引
- サービス拒否
- 展開する
- 設計
- 開発者
- 開発
- DID
- 異なる
- 異なります
- ドメイン
- ダイナミック
- 早い
- エッジ(Edge)
- 奨励する
- 特に
- ETH
- イベント
- イベント
- すべてのもの
- 例
- 交換
- 予想される
- 体験
- 悪用する
- 特徴
- フィールズ
- 最後に
- 名
- 修正する
- 柔軟性
- フロー
- 発見
- フル
- function
- 資金
- 未来
- ゲーム
- GAS
- 与え
- 素晴らしい
- ガイド
- ハンドリング
- ハッシュ
- ハッシュ
- 持って
- 助けます
- ハイ
- 非常に
- 認定条件
- HTTPS
- ハイブリッド
- 識別する
- 影響
- 実装する
- 重要
- インポート
- 含まれました
- 含めて
- 増える
- 増加した
- info
- 情報
- インフラ
- 洞察
- 意図
- 関心
- インタフェース
- 関係する
- 問題
- IT
- 大
- より大きい
- つながる
- 主要な
- 図書館
- 限定的
- LINE
- 流動性
- リストされた
- リスト
- ローカル
- 見
- 検索
- 主要な
- メーカー
- 作成
- 市場
- Mempool
- ミラー
- 最も
- 一番人気
- すなわち
- 新しい特徴
- 代替不可能な
- 置き換え不可能なトークン
- 公式
- 開いた
- 業務執行統括
- オプション
- オラクル
- 注文
- 受注
- その他
- 所有者
- パターン
- 人気
- 現在
- 予防
- ブランド
- 価格設定
- プライベート
- プロセス
- プロジェクト
- プロジェクト(実績作品)
- 保護
- 提供します
- は、大阪で
- 代理
- 公共
- パブリッシュ
- 購入
- 品質
- 上げる
- 現実
- 減らします
- 依存
- レポート
- レポート
- 倉庫
- 要件
- REST
- 結果
- 収益
- レビュー
- リスク
- ラウンド
- ラン
- SDDK
- セキュリティ
- サービス
- セッションに
- shared
- 株式
- シフト
- 同様の
- 簡単な拡張で
- 小さい
- スマート
- スマート契約
- So
- 固い
- スパム
- 特に
- 支出
- Spot
- start
- 都道府県
- ステートメント
- 米国
- Status:
- ストレージ利用料
- 提出された
- 成功
- 成功した
- 首尾よく
- サポート
- サポート
- 表面
- スイッチ
- ターゲット
- test
- テスト
- テスト
- 介して
- 全体
- 時間
- 一緒に
- トークン
- トークン
- 追跡する
- 追跡
- トランザクション
- 信頼
- ユニーク
- 更新版
- us
- 使いやすさ
- USD
- USDT
- users
- ユーティリティ
- 値
- 詳しく見る
- 視認性
- wait
- この試験は
- ホワイトリスト
- 以内
- 無し
- 仕事
- 価値
- ゼロ