# Day5 | mabuk担当分 ## [M-02] ガス不足による引き出し失敗 ### ■ カテゴリー ガス欠 ### ■ 条件 引き出すトークンの数を送信するだけのガスが足りない ### ■ ハッキングの詳細 トークンの数だけループを回して引き出しを行っている。 転送されるトークンの数に制限はない。 ガスの必要量も変わることがある。 つまり資産が転送中に多くのガスを使用する場合提供された資産では権利を行使できないかもしれない。オプションの買い手はプレミアムを支払っているのに、権利を行使できずアセットを手に入れられない。 ### ■ 修正方法 ひとつの取引でのトークンの上限を設けるかループを回さず一度で送信できるようにする ## [M-05] fillOrder()とexercise()によるコントラクトに送られたEtherを永遠にロックする可能性 ### ■ カテゴリー GOX ### ■ 条件 結果的にEtherを必要としない処理となる しかしEtherを送れる ### ■ ハッキングの詳細 payableの関数内でEtherが不要なコードパスがある。このコードパスをEtherを付けて実行できるが、そのEtherはコントラクト内にロックされるだけ ### ■ 修正方法 payableの関数内でEtherが不要なコードパス3箇所に `require(0 == msg.value)` を加える 例 ```solidity // before if (order.isLong{ ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium); } else { // after if (order.isLong{ require(0 == msg.value); ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium); } else { ``` ## [M-08] 関数の重複でerc20AssetsやbaseAssetがERC20ではなくERC721になってしまう ### ■ カテゴリー トークン変異 ### ■ 条件 baseAssetやERC20をtransferFromする ### ■ ハッキングの詳細 ERC721のtransferFrom()が呼ばれることでbaseAseetとERC20がERC721として処理されてしまう。 その事によりERC721として扱われるようになり、権利行使や引き出しが行えなくなる ### ■ 修正方法 ERC721, ERC20それぞれのWLを用意する。 WLを元に明確にERC721かERC20どちらのtransferFrom()を使うか振り分ける ## [M-11] コントラクトオーナーによって手数料の書き換えがユーザーの同意無しで行える ### ■ カテゴリー プロジェクトの信頼 ### ■ 条件 コントラクトオーナーであればはいつでも誰の許可も不要 ### ■ ハッキングの詳細 利用者は1%の手数料だということで、オプション取引をしていた。 しかし、いつの間にかその手数料を3%にしていた。 1%だと思って権利行使したら3%取られていた。 ということがありえる作りになっている。 ### ■ 修正方法 手段1: 手数料はハードコードする 手段2: 手数料が変更された時間を記録しておく(修正ではないが少なくとも今よりは多少マシ) 手段3: 時間と手数料をペアで記録しておき、手数料を変更しても変更前の取引はその当時の手数料で権利行使が行えるようにする ## [M-16] Solidityのバージョン0.8.13には2つの既知の問題がPuttyV2に対してある ### ■ カテゴリー Soitityのバージョン ### ■ 条件 Solidityのバージョン0.8.13 ### ■ ハッキングの詳細 影響範囲 - ABIエンコーディングの脆弱性 - hashOrder() - hashOppositeOrder() - インラインアセンブリのメモリ副作用に関する最適化のバグに関連する脆弱性 - 継承しているopenzeppelinがインラインアセンブリを利用 - 継承しているsolmateがインラインアセンブリを利用 ### ■ 修正方法 Solidityのバージョンを0.8.15に上げる ## [M-14] 注文キャンセルのフロントランニングとオフチェーンDBへの依存 ### ■ カテゴリー フロントランニング ### ■ 条件 2つある 1. キャンセル関数を呼ぶと、それをトリガーにしてフロントランニング出来る 2. キャンセル関数を呼んでも、その関数はオフチェーンDBが必要なためオフチェーンDBが落ちてるとキャンセルとならない。そしてその注文データをなんらかの形で保持しているユーザーには注文を約定できてしまう ### ■ ハッキングの詳細 #### 1.キャンセルの前に約定を差し込める ブロック内にキャンセルが発生したら、それよりも前に約定をさしこみ、結果的にキャンセルできず、約定される。 #### 2.キャンセル出来るはずの注文がキャンセルできない キャンセルしたつもりが、オフチェーンDBが落ちているなどの原因でキャンセルに必要なデータを取り出せずキャンセルが出来なかった場合、他のユーザーがその注文を約定できてしまう。 他のユーザーはオフチェーンのDBではなく、別のところに保持していおいたデータを利用して注文を約定している。 ### ■ 修正方法 cancel関数とは別に、すべての注文をキャンセル出来るようにする。例えばすべての注文のキャンセルはオフチェーンDB使わず単純に注文カウンターなどの状態変数を変更することで全注文をキャンセルとして扱うなど。 ## [H-02] 元の注文とカウンターオファーの注文の2つが約定されてしまう ### ■ カテゴリー フロントランニング ### ■ 条件 acceptCounterOfferが発生したとき必ず ### ■ ハッキングの詳細 acceptCounterOfferが発生したときにフロントランニングでfillOrderを挟み込む(どうやるのかは分かっていません)。そうすとfillOrderもacceptCounterOfferも成功してしまう。 ### ■ 修正方法 以下などの方法で約定済みの注文は、キャンセルは失敗するようにする ```solidity= //before function cancel(Order memory order) public { require(msg.sender == order.maker, "Not your order"); //after function cancel(Order memory order) public { require(msg.sender == order.maker, "Not your order"); require(_ownerOf[uint256(orderHash)] == 0, "約定済みです") ```