LoginSignup
9
10

More than 5 years have passed since last update.

警告メッセージを改善することの効果 - WordPress のwpdb::prepareメソッドにおける実践(PHP5 セキュリティウィザード認定)

Last updated at Posted at 2016-07-04

警告メッセージを改善することの効果 - WordPress のwpdb::prepareメソッドにおける実践

水野史土 (レスキューワーク株式会社 )
https://rescuework.nagoya/

PHP技術者認定機構は、初級試験、上級試験、ウィザード試験を行っている。ウィザード試験は論文形式で審査される。ウィザード合格のための工夫については -> https://rescuework.nagoya/blog/php2015

はじめに

本論文は、PHP技術者認定機構 (http://www.phpexam.jp/) の実施する公開論文審査「PHP5 技術者認定ウィザード2015」に提出し、ウィザード認定された論文である。2016年1月31日に提出され、公開で審査された。その結果、可30票、不可3票、棄権8票で合格となった。

概要

ソフトウェア開発中に、適切でないコードを書いてしまうことがある。この場合、警告メッセージが表示されることにより、アプリケーション開発者にフィードバックが与えられる。表示される警告メッセージが、具体的で修正点を明示した内容であれば、アプリケーション開発者が理解を深め、適切な修正をする手助けになる。
本論文では、 PHP製ソフトウェアの中でも利用者の多いWordPressを題材に取り上げる。WordPressには、安全にSQLを組み立てることができるwpdb::prepareメソッドが用意されている。wpdb::prepareメソッドが適切に使用されないとSQLインジェクションの危険があるため、wpdb::prepareメソッドが適切に使われることは重要である。旧来のWordPressでは、警告メッセージは構文の不備を示すものであった。これを、具体的で修正点を明示した警告メッセージを表示するように、筆者が改善を提案し、取り入れられた。
インターネット上の掲示板での回答を調査した結果、改善前の警告メッセージには不適切な修正を促す回答が見られたが、改善後の警告メッセージには不適切な修正を促す回答が見られなかった。
警告メッセージを出力する処理の速度を測定したが、他のセキュリティ対策用関数の処理速度と比較して遅くはなかった。

目次

  1. SQLインジェクション、プレースホルダ、wpdb::prepare
  2. 著者による警告メッセージの改善
  3. インターネット上の掲示板での回答の調査
  4. パフォーマンスとのトレードオフ
  5. まとめ

1. SQLインジェクション、プレースホルダ、wpdb::prepare

ウェブサイトでデータベースを使うケースは多い。データベースを操作するときはSQLと呼ばれる言語を用いる。SQLを組み立てるコードに脆弱性があった場合、管理者が意図していないSQLを攻撃者が実行できる。このような攻撃をSQLインジェクションと呼ぶ。
たとえば、SQLを組み立てるときに、

(1)
$sql = "SELECT * FROM user WHERE name = '" . $username . "'";

と書いたとする。コード(1)では文字列連結でSQLを組み立てており、もし、パラメータ$username にSQLの命令が入っていた場合、意図しないSQL文が生成される。実際、$username の値が

(2)
$username = "Alice' or 1 = '1";
$sql = "SELECT * FROM user WHERE name = '" . $username . "'";

だった場合、生成されるSQLは

(3)
SELECT * FROM user WHERE name = 'Alice' or 1 = '1'

となる。このSQLが実行されると、「or 1 = '1'」という記述は常に真となるため、すべてのユーザーを取得してしまう。
SQLインジェクションを防ぐ方法の一つに、$usernameを適切にエスケープ処理(特殊な意味を持つ記号を置換する処理)する、という方法がある。こうすることにより、意図しないSQLを生成されることが防げる。
別の方法として、プレースホルダを使う方法がある。この場合は、コード(4)のように記述する。

(4)
$statemant = $db->prepare( "SELECT * FROM user WHERE name = ?" );
$statemant->bindParam( 1, $username, PDO::PARAM_STR );
$statemant->execute();

コード(4)ではPHPに用意されているPDO(PHP Data Object)を使用した。
プレースホルダを使う場合、

  1. SQLの構文を確定し、データベースエンジンに送信する
  2. パラメータをデータベースエンジンに送信する
  3. SQLを実行する

という処理が行われる。SQL構文確定時に変数$usernameを扱わないため、不正なSQLは生成されない。
本論文では、プレースホルダを使う方法を取り上げる。

WordPressは動的プレースホルダ

コード(4)のように、SQL構文とパラメータを別個にデータベースエンジンに送信する方式を静的プレースホルダと呼ぶ。1 SQL構文にパラメータが入らないため、原理的にSQLインジェクションが起きない仕組みである。
一方で、プレースホルダを使うものの、アプリケーション側でパラメータをバインドする処理を行い、作成したSQLをデータベースエンジンに送信する方式もある。この方式を動的プレースホルダと呼ぶ。アプリケーションがSQLを組み立てる実装にバグがあると、SQLインジェクションの危険がある。このため動的プレースホルダを使う場合は、アプリケーションの実装を確認しなければならない。
WordPress2は動的プレースホルダを採用しており、アプリケーションでSQL組み立てを行っている。このため、アプリケーション開発者は、WordPressのSQL組み立てを理解した上で使用しなければ危険である。
WordPressのSQL組み立ては、wpdbクラスの prepareメソッドで行われている。3

(5) 
    public function prepare( $query, $args ) {
        if ( is_null( $query ) )
            return;

        // This is not meant to be foolproof -- but it will catch obviously incorrect usage.
        if ( strpos( $query, '%' ) === false ) {
            _doing_it_wrong( 'wpdb::prepare', sprintf( __( 'The query argument of %s must have a placeholder.' ), 'wpdb::prepare()' ), '3.9' );
        }

        $args = func_get_args();
        array_shift( $args );
        // If args were passed as an array (as in vsprintf), move them up
        if ( isset( $args[0] ) && is_array($args[0]) )
            $args = $args[0];
        $query = str_replace( "'%s'", '%s', $query ); // in case someone mistakenly already singlequoted it
        $query = str_replace( '"%s"', '%s', $query ); // doublequote unquoting
        $query = preg_replace( '|(?<!%)%f|' , '%F', $query ); // Force floats to be locale unaware
        $query = preg_replace( '|(?<!%)%s|', "'%s'", $query ); // quote the strings, avoiding escaped strings like %%s
        array_walk( $args, array( $this, 'escape_by_ref' ) );
        return @vsprintf( $query, $args );
    }

「プレースホルダのあるSQL」を第一引数にとり、「SQLに挿入するパラメータ」を第二引数以降にとる。パラメータが複数の場合は

第二引数に加えて第三引数、第四引数、、、を記述する
第二引数に配列を渡す

かのいずれかが可能。wpdb::prepareメソッドの処理は、

  1. 入力が適切かどうか判定する
  2. 「SQLに挿入するパラメータ」が配列かどうか判定する
  3. 「プレースホルダのあるSQL」のクォート記号を調整する
  4. 「SQLに挿入するパラメータ」を「プレースホルダのあるSQL」へ埋め込む

となっている。
使い方は、

(6)
$wpdb->prepare( "SELECT * FROM table WHERE id = %d", $id );

のように、プレースホルダのあるSQL「SELECT * FROM table WHERE id = %d」とSQLに挿入するパラメータ「$id」を同時に指定する。このため、wpdb::prepareメソッドは引数が最低二つ必要となる。

適切でない使い方への警告

wpdb::prepareメソッドを使うと、WordPress自体にバグが無い限り安全にSQLを組み立てられる。しかし、適切に使われなければ、wpdb::prepareメソッドのセキュリティ効果は発揮されない。wpdb::prepareメソッドの適切でない使い方としては、

(7)
$wpdb->prepare( "SELECT * FROM table WHERE id = $id" );

がある。4
コード(7)のようなコードが書かれた場合は警告するのが好ましい。実際、このような適切でない使い方をしている場合には、警告メッセージ(8)が出る。5

(8)
Warning: Missing argument 2 for wpdb::prepare(), called in 〜(ファイル名)

しかし、警告メッセージ(8)は、不親切であると筆者は考える。
警告メッセージ(8)は、「引数が不足である」という内容である。警告メッセージが表示された場合、アプリケーション開発者が適切な対処を行うことが期待されている。たしかに、アプリケーション開発者の不注意が原因だった場合、この警告メッセージを理解するであろう。しかし、必ずしもアプリケーション開発者が警告メッセージを手正しく理解できるとは限らない。なぜなら、この警告メッセージは、構文上の引数不足を指摘するに過ぎないからである。
もし、wpdb::prepareメソッドの使い方を正しく理解していないアプリケーション開発者がいた場合、適当な値を第二引数として渡す、といった表面的な対処で警告メッセージを回避するかもしれない。たとえばコード(9)のような対処である。

(9)
$wpdb->prepare( "SELECT * FROM table WHERE id = $id", '' );

コード(9)では構文上は引数が二つになったため、警告メッセージは表示されなくなるが、wpdb::prepareメソッドは役に立たない。6この警告メッセージは、アプリケーション開発者の理解度向上には寄与しにくい、という問題がある。

2. 著者による警告メッセージの改善

wpdb::prepareメソッドの不適切な使い方に対して「引数が不足である」という警告メッセージを提示したところ、表面的な対処が行われてしまうケースがあった。「警告メッセージで修正すべき点が明示されていない」ことが原因の一つであると筆者は考えた。そこで、著者は、wpdb::prepareメソッドのチェック方法および警告メッセージを変更することを提案した。
著者が提案したのは「第一引数にプレースホルダがあるかどうかをチェックする」という方法で、警告メッセージは「wpdb::prepare()の第一引数にはプレースホルダが必要だ」というものである。wpdb::prepareメソッドの処理に即した内容であり、間違いの原因を明示したものである。
構文上の引数不足を指摘する警告メッセージが提示された場合、アプリケーション開発者がプレースホルダについて調べることは期待しにくい。一方、著者の提案した警告メッセージを提示された場合、プレースホルダに関する修正が必要なことが明確になっているため、アプリケーション開発者はプレースホルダについて学習する機会を得る。警告メッセージで修正すべき点を明確にすることにより、表面的な修正でなく、適切な修正方法を学び、実行することが期待できる。

WordPress3.9以降で導入

著者は、WordPressの開発状況が公開されている https://core.trac.wordpress.org/ に「wpdb::prepare()の第一引数にはプレースホルダが必要だ」という警告メッセージを提案した。Andrew Nacin氏によるアドバイスとメッセージ文言の調整を経た後、WordPress3.9で導入された。7 wpdb::prepareメソッドに、コード(10)の箇所が追加された。

(10)
        // This is not meant to be foolproof -- but it will catch obviously incorrect usage.
        if ( strpos( $query, '%' ) === false ) {
            _doing_it_wrong( 'wpdb::prepare', sprintf( __( 'The query argument of %s must have a placeholder.' ), 'wpdb::prepare()' ), '3.9' );
        }

3. インターネット上の掲示板での回答の調査

警告メッセージとその対処法について、コミュニティでどのような解決が行われているかを、インターネット上の掲示板wordpress.stackexchange.comとstackoverflow.comで調査した。8 wordpress.stackexchange.comとstackoverflow.comはユーザーが質問や回答をする場であり、wordpressとタグ付けされた質問はwordpress.stackexchange.comで約65,000件、stackoverflow.comで約85,000件ある。調査は2016年1月30日午前10時時点でのものである。
■ wordpress.stackexchange.comを「Warning: Missing argument 2 for wpdb::prepare()」で検索した結果は4件あった。

http://wordpress.stackexchange.com/questions/76072/wpdb-prepare-warning-in-wordpress-3-5
http://wordpress.stackexchange.com/questions/117212/after-wordpress-update-to-3-5-i-get-many-errors-in-plugin-wpdbprepare
http://wordpress.stackexchange.com/questions/131113/wpdb-in-php-5-5
http://wordpress.stackexchange.com/questions/156972/missing-argument-2-for-wpdbprepare-issue

4件とも回答は適切であった。

■ stackoverflow.comを「Warning: Missing argument 2 for wpdb::prepare()」で検索した結果は15件あった。

http://stackoverflow.com/questions/13955416/warning-missing-argument-2-for-wpdbprepare
http://stackoverflow.com/questions/14180208/why-am-i-getting-this-error-after-installing-a-new-theme-on-my-wordpress-site
http://stackoverflow.com/questions/14301604/wordpress-error-after-upgrading-to-version-3-5
http://stackoverflow.com/questions/14369157/missing-argument-2-for-wpdbprepare-error
http://stackoverflow.com/questions/14609384/php-warning-missing-argument-2-for-wpdbprepare-wordpress-3-5
http://stackoverflow.com/questions/14616869/php-warning-missing-argument-2-for-wpdbprepare-wordpress-3-5-updated
http://stackoverflow.com/questions/14691427/php-warning-missing-argument-2-for-wpdbprepare
http://stackoverflow.com/questions/15842775/wpdb-select-table-no-arguments/15842871#15842871
http://stackoverflow.com/questions/18092463/warning-missing-argument-2-for-wpdbprepare-plugin-issue
http://stackoverflow.com/questions/20218561/i-need-a-function-to-append-data-from-one-table-to-another-table
http://stackoverflow.com/questions/21410341/php-warning-missing-argument-2-for-wpdbprepare-wpml-plugin
http://stackoverflow.com/questions/27558182/warning-missing-argument-2-for-wpdbprepare
http://stackoverflow.com/questions/29234945/wordpress-missing-argument-2-for-wpdbprepare
http://stackoverflow.com/questions/32493564/writing-a-wordpress-plugin-trying-to-retrieve-data-from-customized-table
http://stackoverflow.com/questions/34613435/warning-missing-argument-2-for-wpdbprepare-on-theme-functions

15件中9件は回答は適切であった。

http://stackoverflow.com/questions/14301604/wordpress-error-after-upgrading-to-version-3-5
では、プラグイン開発者に修正を促す回答であった。

下記の5件については、不適切な回答や解決になっていた。

http://stackoverflow.com/questions/13955416/warning-missing-argument-2-for-wpdbprepare
では、4つの回答があり、最高評価の回答は
error_reporting(E_ALL & ~(E_NOTICE|E_WARNING));
と、エラーを表示しなくする対処であった。
残りの3つの回答は、
@ini_set('display_errors', 0);
add "" as parameter after query
@ini_set('display_errors', 0);
と、エラーを表示しなくする対処が2件、ダミーの引数を与えてエラーを消す対処が1件あった。

http://stackoverflow.com/questions/14616869/php-warning-missing-argument-2-for-wpdbprepare-wordpress-3-5-updated
では、2つの回答があり、最高評価の回答は適切なものであった。しかし残りの回答は、
@ini_set('display_errors', 0);
と、エラーを表示しなくする対処だった。

http://stackoverflow.com/questions/18092463/warning-missing-argument-2-for-wpdbprepare-plugin-issue
では、質問者自身が「I added null as a second argument to the function」という不適切な解決を提示していた。

http://stackoverflow.com/questions/20218561/i-need-a-function-to-append-data-from-one-table-to-another-table
では、PDOと混同して、「$wpdb->prepare("...")」をOKとする回答があった。

http://stackoverflow.com/questions/21410341/php-warning-missing-argument-2-for-wpdbprepare-wpml-plugin
では、2つの回答があり、最高評価の回答は適切なものであった。しかし残りの回答は、
@ini_set('display_errors', 0);
と、エラーを表示しなくする対処だった。

■ wordpress.stackexchange.comを「The query argument of wpdb::prepare() must have a placeholder.」で検索した結果は4件あった。

http://wordpress.stackexchange.com/questions/142204/plugins-reverting-themself-to-older-versions
http://wordpress.stackexchange.com/questions/149711/wpdbprepare-was-called-incorrectly
http://wordpress.stackexchange.com/questions/165412/incorrect-use-of-wpdbprepare
http://wordpress.stackexchange.com/questions/180546/fixing-wpdb-get-results-and-wpdb-prepare

4件中3件は回答は適切であった。

http://wordpress.stackexchange.com/questions/142204/plugins-reverting-themself-to-older-versions
では、プラグインの停止を促す回答であった。

■ stackoverflow.comを「The query argument of wpdb::prepare() must have a placeholder.」で検索した結果は1件あった。

回答は適切なものであった。

調査結果をまとめると、下記の表のようになる。

質問数 適切な回答 不適切な回答のみ 不適切な回答を含むもの その他
旧来の警告メッセージ 19 13 1 4 1
著者の提案した警告メッセージ 5 4 0 0 1

サンプル数が少ないものの、旧来の警告メッセージでは「エラーを抑制する」という表面上の対処を提示する回答のような不適切な回答が有り、著者の提案した警告メッセージについては、そのような回答は無かった。

4. パフォーマンスとのトレードオフ

「wpdb::prepare()の第一引数にはプレースホルダが必要だ」という警告メッセージは、セキュリティ上重要ではある。しかし、wpdb::prepareメソッド内で引数のチェックを行い、この警告メッセージを出力することにより、処理速度が遅くなると考えられる。
本節では、処理速度について検証し、警告メッセージによる処理速度への影響が許容範囲かどうかを考察する。
wpdb::prepareメソッド内で引数のチェックして警告メッセージを表示する/しない
の処理速度の差を調べる。また比較対象として
esc_html関数を使う/使わない
の処理速度の差を調べる。
速度の検証には、MacBook Air 11inch (mid 2010) 上の MAMP PRO で、PHP5.4.10、WordPress4.4.1を使用した。インストール直後の状態で、標準テーマtwentysixteenのindex.phpにコードを記述した。コードは各々5回実行し、中央値を採用した。
■ 警告する場合の処理速度
twentysixteenのindex.phpに、下記のコード(11)を追加して実行した。9

(11)
<?php
$start = microtime( true );
for ( $i=0; $i<10000; $i++ ) {
    $wpdb->prepare( "SELECT option_value FROM $wpdb->options WHERE option_name = %s LIMIT 1", 'siteurl' );
}
$end = microtime( true );
echo $end - $start;
?>

5回実行した結果は、下記の通りである。
0.564436912537
0.575039148331
0.568149805069
0.563189983368
0.560966968536

■ 警告しない場合の処理速度
wp-includes/wp-db.php の prepareメソッドの定義から、行頭に「※」をつけた4行を削除して、コード(11)を実行した。

(12)
    public function prepare( $query, $args ) {
        if ( is_null( $query ) )
            return;

※     // This is not meant to be foolproof -- but it will catch obviously incorrect usage.
※     if ( strpos( $query, '%' ) === false ) {
※         _doing_it_wrong( 'wpdb::prepare', sprintf( __( 'The query argument of %s must have a placeholder.' ), 'wpdb::prepare()' ), '3.9' );
※     }

        $args = func_get_args();
        array_shift( $args );
        // If args were passed as an array (as in vsprintf), move them up
        if ( isset( $args[0] ) && is_array($args[0]) )
            $args = $args[0];
        $query = str_replace( "'%s'", '%s', $query ); // in case someone mistakenly already singlequoted it
        $query = str_replace( '"%s"', '%s', $query ); // doublequote unquoting
        $query = preg_replace( '|(?<!%)%f|' , '%F', $query ); // Force floats to be locale unaware
        $query = preg_replace( '|(?<!%)%s|', "'%s'", $query ); // quote the strings, avoiding escaped strings like %%s
        array_walk( $args, array( $this, 'escape_by_ref' ) );
        return @vsprintf( $query, $args );
    }

5回実行した結果は、下記の通りである。
0.554287910461
0.540683984756
0.54522895813
0.542409181595
0.550657987595

不適切な使い方を警告する場合、中央値は0.564436912537
不適切な使い方を警告しない場合、中央値は0.54522895813
その差は0.0192079544である。

■ esc_htmlの処理速度
処理速度の比較として、esc_htmlの処理速度を調査した。esc_htmlは、出力時に文字コードのチェックや特殊文字のエスケープ処理等を行う関数で、この関数を適切に使用すればクロスサイトスクリプティングを防ぐことができる。下記のコード(13)を実行した。

(13)
<?php
$start = microtime( true );
for ( $i=0; $i<10000; $i++ ) {
    $data = esc_html( '<script>alert(1)</script>' );
}
$end = microtime( true );
echo $end - $start;
?>

5回実行した結果は、下記の通りである。
0.763100862503
0.766307830811
0.760141849518
0.764425039291
0.765257120132

■ esc_htmlを使わない場合の処理速度
比較のため、esc_htmlを使わない場合の処理速度を測定するコード(14)を実行した。

(14)
<?php
$start = microtime( true );
for ( $i=0; $i<10000; $i++ ) {
    $data = '<script>alert(1)</script>';
}
$end = microtime( true );
echo $end - $start;
?>

5回実行した結果は、下記の通りである。
0.00473594665527
0.00469493865967
0.00479292869568
0.0047779083252
0.00474905967712

esc_htmlを使う場合、中央値は0.763100862503
esc_htmlを使わない場合、中央値は0.00474905967712
その差は0.75835180282である。

wpdb::prepareメソッドで、警告するコストは許容範囲

wpdb::prepareメソッドで、不適切な使い方を警告する場合、処理速度の増加というコストが発生する。しかし、警告することにより増加した時間は、一万回実行して約0.02秒である。一方、esc_htmlを使う場合と使わない場合の処理速度の差は一万回実行して約0.7秒である。
esc_htmlは、処理速度は速くないが、セキュリティ上重要な関数である。また、wpdb::prepareメソッドも、セキュリティ上重要な関数である。wpdb::prepareメソッドで不適切な使い方を警告する場合でも、esc_htmlの処理速度と比較して遅い訳ではない。この点から、wpdb::prepareメソッドで、警告するコストは、セキュリティ維持のためのコストとして許容範囲であると考えられる。

5. まとめ

WordPressのwpdb::prepareメソッドでは、旧来の警告メッセージが構文不備の指摘であったのに対し、著者はprepareメソッドの処理内容に踏み込んだ警告メッセージを提案した。この提案はWordPressコアに取り入れられた。
インターネット上の掲示板において、wpdb::prepareメソッドについて質問回答が行われている。警告メッセージ変更前と変更後の質問回答を調査したところ、警告メッセージ変更前には不適切な修正方法を提示する回答が見られたが、警告メッセージ変更後は不適切な修正方法を提示する回答が見られなかった。警告メッセージを改善することで、「不適切なコードが適切に修正されない」という危険性を減らすことができる。
警告メッセージを出力するコストも検証した。クロスサイトスクリプティング対策を行う関数esc_htmlの実行コストと比較して遜色は無いため、セキュリティ維持のためのコストとして許容範囲と考えられる。


  1. 「静的プレースホルダ」「動的プレースホルダ」という用語は、IPA 「安全なSQLの呼び出し方」を元にした。https://www.ipa.go.jp/security/vuln/websecurity.html より入手可能。 

  2. WordPressは、PHPで記述されているウェブサイト構築ツールであり、データベースを使うシステムである。 https://wordpress.org/ より入手可能。 

  3. wp-includes/wp-db.php に定義されている 

  4. https://make.wordpress.org/core/2012/12/12/php-warning-missing-argument-2-for-wpdb-prepare/ で報告されている。 

  5. 警告メッセージを表示する実装は WordPress3.5 で追加された。 

  6. 実際に、このような対処を行ったプラグインがある。たとえば、WordPress HTTPS ver.3.3.6https://wordpress.org/plugins/wordpress-https/ 

  7. https://core.trac.wordpress.org/ticket/25604 を参照 

  8. WordPressにもフォーラム https://wordpress.org/support/ があるが、このフォーラムは筆者が参加しているため、客観性の観点から調査対象にしなかった。 

  9. コード(11)では、処理速度計測対象のwpdb::prepareメソッドを、for文で一万回繰り返し実行している。microtime関数でその前後の時刻を記録し、その差分を実行時間としている。後述のコード(13)、コード(14) も同様。 

9
10
2

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
9
10