Feature request - Check code security

[Potato1682]

概要

セキュリティ脆弱性が明確に確認できていないため、セキュリティを次の項などのやり方で確認することをお勧めします。

どのようなものが「脆弱性」であるか

• [x] 安全でないコードの実行 ポインターなどアンセーフなコードの実行は危険な場合が多いです。基準はC#のunsafeブロック範囲が良いと思われます。
• [x] 安全ではない通信方法 暗号化など安全ではないパケット送受信は(ローカルでも)危険であり、通信内容を改竄される可能性があります。
• [x] 危険なメモリ管理 Javaは他のオブジェクト指向言語と比べて、メモリ管理をプログラム側からかなり自由に行うことができます。 そのため、ファイナライズを怠ったり危険なメモリ管理系関数を実行するとメモリデータに脆弱性が発生する可能性があります。
• [x] バイナリファイルの署名 S/MIMEだとかGnuPGだとかなんでもいいのでバイナリファイルに署名をすることをおすすめします。GPG4Winなら右クリックで出る[More GPGEx options]にてverifyすることができます。
• [x] 依存関係のバージョンを最新にする リリース前でも定期的でもいいのでpom.xmlに記述している依存関係を最新にすることをお勧めします。
• [x] コード内の脆弱性のアドバイスプラグインの使用 JetBrainsなどで提供されているプラグインを使用して脆弱性の発見を素早くしましょう。
  • Snyk Vulnerability Scanning
  • RIPS Security Analysis
  • Synopsys Code Sight

補足

セキュリティ脆弱性が発見された場合は必ずIssueに[security]ラベルをつけて立ててください。 @Potato1682がセキュリティ脆弱性としてVulnerabilities.mdにナンバリングします。

また、こちらにも役立つ情報が載っています。

[PeyaPeyaPeyang]

まず、安全でないコードの実行に関しては、、、善処します。 安全ではない通信方法については、PlayerConnectionクラスのsendPacketクラスを使用しているため、12w17a以降は送信時に自動で暗号化されるはずです。 バイナリファイルの署名については、Bukkit/Spigotの場合は不要と考えられます。 依存関係のバージョンのを最新にする については、DepndBotにより最新に保たれています。 DependBotは、毎日午後8時に自動でトリガーされるようにせっていされています。 コード脆弱性指摘プラグインについては、導入いたします。

[Potato1682]

Issueをチェックボックス形式に変更しました。 危険なメモリ管理についてのコメントをお願い致します。

[PeyaPeyaPeyang]

危険なメモリ管理については、いくつか例があります。 例えば、Commandの引数の長さをありえないくらい大きくしたとしましょう。このとき、どこかしらでエラーが発生します。 クライアントがHackClientなどを使っていない場合、指定できる最大のコマンドの文字数は、256文字です。 ですが、HackClientなどを使用していた場合、この文字数は、Stringの限界までできますが、 Bukkit側でエラーが発生し、プラグイン側に伝達されません。 次に、Int値の最大までNPCを殴り、オーバーフローした場合です。 まぁ、多分物理的に21億回も殴るのは無謀かと思われます。 また、サーバ主がConfig等を使用し、意図的に脆弱性を作成した場合は、 もう開発者としては、「しらんがな」としか言いようがありません。

[Potato1682]

サーバー主視点とまで来たらそりゃ「しらんがな」になるでしょう。 このリポジトリのライセンスはMITライセンスなので、そのような自己責任関係はわざわざ修正するべきではありません。 しかし、あくまで予測ですが@peyang-Celeronさんが想定している範囲以外にも危険であるメモリ管理方法はあると思います。

[PeyaPeyaPeyang]

あー。。。 例えば、HackClientを用いて、アタックパケットを大量に送りつけ、21億を超えた場合、オーバーフローして負の値に行くかもしれませんが、大量のPacketはキックまたはBukkit側で切り落とされるはずです。 また、サーバ主が、NPCのスポーン時間を1時間などにセットした場合、21億回殴ることは可能かと思われます。その場合も、「しらんがな」とはなりますが、READMEなどで言及したほうがいいかもしれません。

[Potato1682]

確かにそれはあるかもしれませんが、それは前述のコメントの通りに言うとそれも「自己責任」となり修正する意味は無くなると思います。

あくまで予測ですが@peyang-Celeronさんが想定している範囲以外にも危険であるメモリ管理方法はあると思います。

と前述したため他の危険な可能性をチェックしてみてください。 全てのチェックが終わったら(チェックボックス関係なく)、このIssueをCloseします。

[PeyaPeyaPeyang]

危険なメモリ管理のチェック完了しました。 今の所、当プラグインの中には、サーバを停止させるような危険なコードは、含まれていないようです。

[Potato1682]

了解しました。まぁ自分が言えるかといったらそうでもないかもしれませんが、脆弱性は見つける前に予想するのも重要ですので、たまに思いついたらテストする感じで行きましょう。クローズします。