静的コード解析ツールは、バグや脆弱性をどの程度検出できるのでしょうか？
以前つくったバグだらけのWebアプリケーションをFindBugsで解析することで、検証してみました。
検証内容
バグだらけのWebアプリケーション(EasyBuggy 1.2.17)が、現在実装済みのバグや脆弱性は以下の68種類です。
障害
- メモリリーク (Javaヒープ領域)
- メモリリーク (Permanent領域)
- メモリリーク (Cヒープ領域)
- デッドロック (Java)
- デッドロック (SQL)
- 完了しないプロセスの待機
- 無限ループ
- リダイレクトループ
- フォワードループ
- JVMクラッシュ
- ネットワークソケットリーク
- データベースコネクションリーク
- ファイルディスクリプタリーク
- スレッドリーク
- 文字化け
- 整数オーバーフロー
- 丸め誤差
- 打ち切り誤差
- 情報落ち
脆弱性
- XSS (クロスサイトスクリプティング)
- SQLインジェクション
- LDAPインジェクション
- コードインジェクション
- OSコマンドインジェクション
- サイズ制限の無いファイルアップロード
- 拡張子制限の無いファイルアップロード
- オープンリダイレクト可能なログイン画面
- ブルートフォース攻撃可能なログイン画面
- 親切過ぎるログインエラーメッセージ
- 危険なファイルインクルード
- 意図しないファイル公開
- CSRF (クロスサイトリクエストフォージェリ)
性能に関する問題
- 正規表現解析による遅延
エラー
- FactoryConfigurationError
- ExceptionInInitializerError
- NoClassDefFoundError
- OutOfMemoryError (Java heap space)
- OutOfMemoryError (Requested array size exceeds VM limit)
- OutOfMemoryError (unable to create new native thread)
- OutOfMemoryError (GC overhead limit exceeded)
- OutOfMemoryError (PermGen space)
- OutOfMemoryError (Direct buffer memory)
- StackOverflowError
- UnsatisfiedLinkError
非チェック例外
- ArithmeticException
- ArrayIndexOutOfBoundsException
- ArrayStoreException
- BufferOverflowException
- BufferUnderflowException
- CannotRedoException
- CannotUndoException
- ClassCastException
- ConcurrentModificationException
- EmptyStackException
- IllegalArgumentException
- IllegalMonitorStateException
- IllegalPathStateException
- IllegalStateException
- IllegalThreadStateException
- ImagingOpException
- IndexOutOfBoundsException
- InputMismatchException
- MissingResourceException
- NegativeArraySizeException
- NoSuchElementException
- NullPointerException
- NumberFormatException
- UnsupportedOperationException
FindBugs 3.0.1はどの程度の問題を検出できるのでしょうか？デフォルトと解析力最大の2つの設定で、FindBugsを実行してみます。
結果
結果は以下のようになりました。
2列目の「結果(デ)」はデフォルトの設定での解析結果で、「結果(大)」は解析力最大の設定での解析結果です。
| バグ | 結果(デ) | 結果(大) |
|---|---|---|
| メモリリーク (Javaヒープ領域) | × | × |
| メモリリーク (Permanent領域) | × | × |
| メモリリーク (Cヒープ領域) | × | × |
| デッドロック (Java) | × | × |
| デッドロック (SQL) | × | × |
| 完了しないプロセスの待機 | × | × |
| 無限ループ | × | × |
| リダイレクトループ | × | × |
| フォワードループ | × | × |
| JVMクラッシュ | × | × |
| ネットワークソケットリーク | × | × |
| データベースコネクションリーク | × | ○ |
| ファイルディスクリプタリーク | × | ○ |
| スレッドリーク | × | × |
| 文字化け | × | × |
| 整数オーバーフロー | × | × |
| 丸め誤差 | × | × |
| 打ち切り誤差 | × | × |
| 情報落ち | × | × |
| XSS (クロスサイトスクリプティング) | × | × |
| SQLインジェクション | × | ○ |
| LDAPインジェクション | × | × |
| コードインジェクション | × | × |
| OSコマンドインジェクション | × | × |
| サイズ制限の無いファイルアップロード | × | × |
| 拡張子制限の無いファイルアップロード | × | × |
| オープンリダイレクト可能なログイン画面 | × | × |
| ブルートフォース攻撃可能なログイン画面 | × | × |
| 親切過ぎるログインエラーメッセージ | × | × |
| 危険なファイルインクルード | × | × |
| 意図しないファイル公開 | × | × |
| 正規表現解析による遅延 | × | × |
| CSRF (クロスサイトリクエストフォージェリ) | × | × |
| FactoryConfigurationError | × | × |
| ExceptionInInitializerError | × | × |
| NoClassDefFoundError | × | × |
| OutOfMemoryError (Java heap space) | × | × |
| OutOfMemoryError (Requested array size exceeds VM limit) | × | × |
| OutOfMemoryError (unable to create new native thread) | × | × |
| OutOfMemoryError (GC overhead limit exceeded) | × | × |
| OutOfMemoryError (PermGen space) | × | × |
| OutOfMemoryError (Direct buffer memory) | × | × |
| StackOverflowError | × | × |
| UnsatisfiedLinkError | × | × |
| ArithmeticException | × | × |
| ArrayIndexOutOfBoundsException | ○ | ○ |
| ArrayStoreException | × | × |
| BufferOverflowException | × | × |
| BufferUnderflowException | × | × |
| CannotRedoException | × | × |
| CannotUndoException | × | × |
| ClassCastException | × | × |
| ConcurrentModificationException | × | × |
| EmptyStackException | × | × |
| IllegalArgumentException | × | × |
| IllegalMonitorStateException | × | × |
| IllegalPathStateException | × | × |
| IllegalStateException | × | × |
| IllegalThreadStateException | × | × |
| ImagingOpException | × | × |
| IndexOutOfBoundsException | × | × |
| InputMismatchException | × | × |
| MissingResourceException | × | × |
| NegativeArraySizeException | × | × |
| NoSuchElementException | × | × |
| NullPointerException | ○ | ○ |
| NumberFormatException | × | × |
| UnsupportedOperationException | × | × |
結果は、68種類のバグに対してデフォルト設定で2種類、解析力最大の設定でも5種類しか検出できませんでした。FindBugsなどの静的解析ツールは、かなり細かい指摘を上げてくる印象があったのですが、予想以上にバグを検出できないようです。
とはいえ、それ以外の指摘を含む全ての指摘の数は、デフォルト設定で6件、解析力最大の設定で144件ありました(コードの行数約3,300行に対して)。
また、オープンリダイレクトの脆弱性は検出できませんでしたが、次のような行でHTTPレスポンススプリッティングの脆弱性を指摘しました。
response.sendRedirect("/openredirect/login" + loginQueryString);
HTTPレスポンススプリッティングは、response.sendRedirect()の引数に改行を含むような文字列を指定すると、改行により別のレスポンスヘッダを埋め込むことができるという脆弱性ですが、ほとんどのサーブレットコンテナ（JettyやTomcatを含む）の新しいバージョンでは、受信したヘッダー値の改行をエスケープしてこの種の攻撃から保護しているため、この指摘は当てはまらないことの方が多いと思います。
※ 注意：今回の検証で検出できたバグであっても、実装方法が異なれば検出できない可能性があります。逆に今回の検証で検出できなかったバグであっても、実装方法が異なれば検出できる可能性があります。100%の確率でNullPointerExceptionが発生するロジックでも、実装の仕方によってはNullPointerExceptionを検出できる場合も、できない場合もあります。
まとめ
今回はこのような結果が出ましたが、意図的に作り込んだバグ以外の箇所で、FindBugsは多くのコード上の問題を指摘してくれました。FindBugsは、単純なコーディングミスの指摘やよりよい実装方法の提案などをしてくれるので、非常に有益なツールであることは確かです。
ただし、前述したHTTPレスポンススプリッティングの件のように、全ての指摘を取り込む必要もありません。逆に不要なコードを追加してしまったり、可読性を下げてしまう可能性もあります。
また、今回の結果から人間によるコードレビューの重要性が高いということが言えます。コードレビュー前の機械的なチェックに静的解析ツールを活用することは有益ですが、最終的にバグをつくりこまないようにするのは人間(優秀なプログラマー)によるコードレビューしかないと思います。
まとめると、
- FindBugsの指摘が無くても、バグは多数潜在している可能性がある
- とはいえ、FindBugsは多くの問題を検出できるため、とても有益
- FindBugsが検出した全ての問題を取り込む必要は無い
- 人間によるコードレビューが重要