前回：やられアプリ BadTodo - 3.8 SQLインジェクション sqlmapを使ってみる - demandosigno
これまで検査方法についてIPAのサイトや徳丸本を参考にしてきましたが、対策方法についても同様です。
「安全なウェブサイトの作り方 - 1.1 SQLインジェクション | 情報セキュリティ | IPA 独立行政法人 情報処理推進機構」
「安全なSQLの呼び出し方」
特にBadTodoはもともと徳丸本の付属資料の一つですので、並行して読んだ方が絶対によいです。
「体系的に学ぶ 安全なWebアプリケーションの作り方 第2版 | SBクリエイティブ」
根本的解決
- SQL文の組み立ては全てプレースホルダで実装する。
- SQL文の組み立てを文字列連結により行う場合は、エスケープ処理等を行うデータベースエンジンのAPIを用いて、SQL文のリテラルを正しく構成する。
- ウェブアプリケーションに渡されるパラメータにSQL文を直接指定しない。
SQL文の組み立ては全てプレースホルダで実装する
SQLには通常、プレースホルダを用いてSQL文を組み立てる仕組みがあります。SQL文の雛形の中に変数の場所を示す記号（プレースホルダ）を置いて、後に、そこに実際の値を機械的な処理で割り当てるものです。ウェブアプリケーションで直接、文字列連結処理によってSQL文を組み立てる方法に比べて、プレースホルダでは、機械的な処理でSQL文が組み立てられるので、SQLインジェクションの脆弱性を解消できます。
プレースホルダに実際の値を割り当てる処理をバインドと呼びます。バインドの方式には、プレースホルダのままSQL文をコンパイルしておき、データベースエンジン側で値を割り当てる方式（静的プレースホルダ）と、アプリケーション側のデータベース接続ライブラリ内で値をエスケープ処理してプレースホルダにはめ込む方式（動的プレースホルダ）があります。静的プレースホルダは、SQLのISO/JIS規格では、準備された文(Prepared Statement)と呼ばれます。
どちらを用いてもSQLインジェクション脆弱性を解消できますが、原理的にSQLインジェクション脆弱性の可能性がなくなるという点で、静的プレースホルダの方が優ります。詳しくは本書別冊の「安全なSQLの呼び出し方」のプレースホルダの項（3.2節）を参照してください。
安全なウェブサイトの作り方 - 1.1 SQLインジェクション | 情報セキュリティ | IPA 独立行政法人 情報処理推進機構
IPA - 別冊：安全なSQLの呼び出し方
詳細は上記資料にお任せすることにして、ここではBadTodoに対して実際に修正してみた例を記載します。
ログイン画面（logindo.php の userid, pwd）に対して
元のコードが入力値を SQL文に直に放り込む形になっている所を、静的プレースホルダを使って書き換えます。
プレースホルダにはコロン:paramや?を使います。PHPは双方使えてBadTodoでも両方が使われていますが今回は?で試します。
PHP: プリペアドステートメントおよびストアドプロシージャ - Manual
PHP: PDO::prepare - Manual
また、プレースホルダにパラメータを割り当て（バインド）するのには bindParam()や bindValue()が使えますが、BadTodoでの他の箇所に合わせてexecute()に配列を渡す形にしました。
PHP: PDOStatement::bindParam - Manual
PHP: PDOStatement::bindValue - Manual
bindParam()とbindValue()とは #PHP - Qiita
注：「安全なSQLの呼び出し方」p.24に『execute メソッドでパラメータの値を与えることもできますが、その場合、パラメータはすべてvarchar 型とみなされ、SQL 実行時に文字列型から実際の型への「暗黙の型変換」が生じます。MySQLの場合、文字列型から数値型への変換時に浮動小数点数型を経由するため、精度が損なわれるなどの不具合の原因になります。このことから、MySQL では、bind_param メソッドで型を明示してバインドする方法を推奨します。』との記載がありました。
logindo.php コメントアウトしている部分を修正例に書き換えてください。
<?php ～（中略）～ // $sql = "SELECT id, userid FROM users WHERE userid='$userid'"; $sql = "SELECT id, userid FROM users WHERE userid=?"; // $sth = $dbh->query($sql); $sth = $dbh->prepare($sql); // プリペアドステートメント（静的プレースホルダ）でクエリを準備する $sth->execute(array($userid)); // プリペアドステートメントで値を取得して実行する $row = $sth->fetch(PDO::FETCH_ASSOC); $sth = null; if (!empty($row)) { // $sql = "SELECT id, userid, super FROM users WHERE userid='$userid' AND pwd='$pwd'"; $sql = "SELECT id, userid, super FROM users WHERE userid=? AND pwd=?"; // $sth = $dbh->query($sql); $sth = $dbh->prepare($sql); // 準備する $sth->execute(array($userid, $pwd)); // 実行する
また common.phpの78行目からの部分にsetAttribute()を１行追記してください。
PHP: PDO::setAttribute - Manual
<?php ～（中略）～ function dblogin() { $dbhost = isset($_ENV['REDIRECT_MYSQL_HOST']) ? $_ENV['REDIRECT_MYSQL_HOST'] : '127.0.0.1'; $dbh = new PDO("mysql:host=$dbhost;dbname=todo", 'root', 'wasbook'); $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); // 追記。falseで静的プレースホルダを使用する。デフォルト値はtrue $dbh->query("SET NAMES utf8"); return $dbh; }
確認
BadTodo - 3.1 SQLインジェクション 認証の回避で試した単純なシングルクォーテーション'の入力で確認してみます。
ログイン実行。SQLのエラーは表示されませんでした。
この時のクエリを確認してみると先にPrepareでひな型が作成され、Executeで入力値がはめ込まれていますが、'がバックスラッシュでエスケープされています。
Prepare SELECT id, userid FROM users WHERE userid=? Execute SELECT id, userid FROM users WHERE userid='\''
（文字列に引用符'を含める方法は幾つかあり、シングルクォーテーションを重ねる''方法もあります）MySQL :: MySQL 8.0 リファレンスマニュアル :: 9.1.1 文字列リテラル
静的プレースホルダの場合、プリペアドステートメントを利用した時点でSQL構文が確定し後から構文が変化することがありません。割り当てられる変数は完全な数値定数もしくは文字列定数として扱われます。特殊文字はエスケープ処理されます。よってパラメータの値がリテラルの外にはみ出す現象が起きず、SQL インジェクションの脆弱性が生じません。
（一方、対策以前のクエリ（下記）ではそのまま入力されるため、２つ目の'で文字列リテラルが終了し、３つ目の'は文字列リテラルをはみ出した状態になり、SQL文として意味を持たないので構文エラーになります）
Query SELECT id, userid FROM users WHERE userid='''
パスワードの方も確認
id=admin パスワード='OR'1で試します。（デフォルトではパスワード６桁までしか受け付けない仕様のため）
修正前のSQLインジェクションが可能な場合。
ログインできました。
クエリは次の通り。
Query SELECT id, userid FROM users WHERE userid='admin' Query SELECT id, userid, super FROM users WHERE userid='admin' AND pwd=''OR'1'
修正後。ログインできません。
Prepare SELECT id, userid FROM users WHERE userid=? Execute SELECT id, userid FROM users WHERE userid='admin' Close stmt Prepare SELECT id, userid, super FROM users WHERE userid=? AND pwd=? Execute SELECT id, userid, super FROM users WHERE userid='admin' AND pwd='\'OR\'1'
補足。動的プレースホルダとShift_JISによるSQLインジェクション
先の修正で common.php に「静的プレースホルダを使うように」という指示を１行追加しました。
$dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); // 追記。falseで静的プレースホルダを使用する。デフォルト値はtrue
この記載がない場合はデフォルトの「動的プレースホルダ」が使用されます。
動的プレースホルダは準備された文(Prepared Statement)とは異なり、プレースホルダを利用するものの、パラメータのバインド処理をデータベースエンジン側で行うのではなく、アプリケーション側のライブラリ内で実行する方式です。 ～（中略）～
これを俗に、「クライアントサイドのプリペアドステートメント」と呼ぶことがありますが、JIS/ISO で規定された意味での「準備された文(Prepared Statement)」ではないので注意が必要です。
IPA - 別冊：安全なSQLの呼び出し方 p.11
パラメータのバインド処理をデータベースエンジン側で行うのではなくアプリケーション側で実行する場合、文字エンコードにShift_JISを使うとSQLの生成においてUnicodeからShift_JISへの変換が発生します。
（PHP/MySQL/Shift_JISの組み合わせだけではなく様々なパターンがありますのでIPAの資料を適宜参照してください）
ここでは上記資料「安全なSQLの呼び出し方 p.32」A.2. Shift_JISによるSQLインジェクションを試してみます。
まずブラウザChromiumに拡張機能を入れ、Webサイトのエンコーディングを変更できるようにしておいてください。=>https://chromewebstore.google.com/detail/charset/oenllhgkiiljibhfagbfogdbchhdchml?hl=ja&utm_source=ext_sidebar
そして common.php の$dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);をコメントアウトして当初の状態に戻します。また文字セットにutf8ではなくsjisを指定します。
<?php ～（中略）～ function dblogin() { $dbhost = isset($_ENV['REDIRECT_MYSQL_HOST']) ? $_ENV['REDIRECT_MYSQL_HOST'] : '127.0.0.1'; $dbh = new PDO("mysql:host=$dbhost;dbname=todo", 'root', 'wasbook'); $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); // $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); // $dbh->query("SET NAMES utf8"); $dbh->query("SET NAMES sjis"); return $dbh; }
先ほどと同じく、まずは単純なシングルクォーテーション'の入力。
SQL文のエラーは表示されませんでした。
この時のクエリを確認してみると静的プレースホルダの際と同様に'がバックスラッシュでエスケープされており問題ないように見えます。ただ Prepare Execute という記載がなくなり、動的プレースホルダになっているとみられます。
Query SELECT id, userid FROM users WHERE userid='\''
次に、先ほど導入した拡張機能を使いブラウザの文字コードを「日本語（Shift_JIS）」に変更します。
PHPコード自体はUTF-8形式で保存されているため文字化けします。
ここでユーザIDに表'; TRUNCATE todos;--と入力しログインボタンをクリックするとテーブルが初期化されます。
実行前 MariaDB [todo]> select * from todos; +----+-------+--------------------------------+------------+------------+------+------+--------------+---------------+------+----------+--------+ | id | owner | todo | c_date | due_date | done | memo | org_filename | real_filename | url | url_text | public | +----+-------+--------------------------------+------------+------------+------+------+--------------+---------------+------+----------+--------+ | 1 | 1 | パソコンを買う | 2023-12-11 | 2023-12-12 | 0 | NULL | NULL | NULL | NULL | NULL | 1 | | 2 | 2 | 依頼の原稿を書く | 2023-12-11 | 2023-12-18 | 0 | NULL | memo.txt | memo.txt | NULL | NULL | 1 | | 3 | 1 | 政府高官との会食アポ | 2023-12-11 | 2023-12-14 | 0 | NULL | NULL | NULL | NULL | NULL | 0 | +----+-------+--------------------------------+------------+------------+------+------+--------------+---------------+------+----------+--------+ 3 rows in set (0.000 sec) 実行後 MariaDB [todo]> select * from todos; Empty set (0.001 sec)
この時のクエリ
Query SET NAMES sjis Query SELECT id, userid FROM users WHERE userid='\\\'; TRUNCATE todos;-- '
なぜこうなるのかについて。
入力値の表'の部分の文字コードは次のようになります。
表のShift_JISコードは0x955cですが下位 8bit の0x5cはUS-ASCII（UTF-8エンコード）ではバックスラッシュに該当します。この文字列に対し、文字エンコーディングを考慮せずにエスケープ処理を施した場合、0x5c がエスケープ対象と解釈されることがあります。
また先の例で見たように、MySQLではシングルクォーテーション(0x27)やバックスラッシュ(0x5c)は、その前にバックスラッシュ(0x5c)を付けることでエスケープされます。\\や\'。よって以下のような並びとなります。
しかしこれをあらためて Shift_JIS として解釈するとこうなります。
これを元の SQL 文に与えると次の文になります。
SELECT id, userid FROM users WHERE userid='表\\'; TRUNCATE todos;--
本来後部の'をエスケープするための2番目の\が1番目の\との組み合わせ\\となってしまったため、後ろの'が有効となりセミコロン以降がリテラルの外にはみ出しSQL文として解釈されます。
このことから、文字エンコーディングに起因する SQL インジェクションの脆弱性が発生しやすい Shift_JIS の使用は避けてください。
（DockerTerminal上ではuserid='\\\'; とバックスラッシュ3本で表示されましたが、badtodo-dbの元のubuntuのlocaleにUTF-8しか入っていないので正しく表示できていないだけだと思われます）
（後ろが0x5cとなる文字は表以外にも複数あります）
静的プレースホルダとShift_JISのパターンを試す
まずデータ初期化用スクリプトを実行し、テーブルを元に戻します。
(Windows(CMD) C:badtodo> .\scripts\init.bat) (Mac / Linux / WSL $ ./scripts/init.sh)
badtodo/docs/usage.md at main · ockeghem/badtodo · GitHub
common.phpにて文字コードはsjisのままsetAttribute(PDO::ATTR_EMULATE_PREPARES, false);とします。
<?php ～（中略）～ function dblogin() { $dbhost = isset($_ENV['REDIRECT_MYSQL_HOST']) ? $_ENV['REDIRECT_MYSQL_HOST'] : '127.0.0.1'; $dbh = new PDO("mysql:host=$dbhost;dbname=todo", 'root', 'wasbook'); $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $dbh->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); // $dbh->query("SET NAMES utf8"); $dbh->query("SET NAMES sjis"); return $dbh; }
そして表'; TRUNCATE todos;--でログイン。
今度は消えません。
MariaDB [todo]> select * from todos; +----+-------+--------------------------------+------------+------------+------+------+--------------+---------------+------+----------+--------+ | id | owner | todo | c_date | due_date | done | memo | org_filename | real_filename | url | url_text | public | +----+-------+--------------------------------+------------+------------+------+------+--------------+---------------+------+----------+--------+ | 1 | 1 | パソコンを買う | 2023-12-11 | 2023-12-12 | 0 | NULL | NULL | NULL | NULL | NULL | 1 | | 2 | 2 | 依頼の原稿を書く | 2023-12-11 | 2023-12-18 | 0 | NULL | memo.txt | memo.txt | NULL | NULL | 1 | | 3 | 1 | 政府高官との会食アポ | 2023-12-11 | 2023-12-14 | 0 | NULL | NULL | NULL | NULL | NULL | 0 | +----+-------+--------------------------------+------------+------------+------+------+--------------+---------------+------+----------+--------+
SET NAMES sjis Execute SET NAMES sjis Close stmt Prepare SELECT id, userid FROM users WHERE userid=? Execute SELECT id, userid FROM users WHERE userid=X'955C273B205452554E4341544520746F646F733B2D2D20'
PrepareとExecuteで実行され、'は27などになり無害となっています。
さらに補足
`PDO::ATTR_EMULATE_PREPARES => false`は必要か？の記事で徳丸さんがコメントされており、
すなわち、以下のようなことは言えるかと思います。
・過去にはプレースホルダのエミュレーションの脆弱性が発見されたことがあったが、最近は報告がないようだ
・過去にあった脆弱性もUnicodeとJIS系エンコーディングを混在した場合のものであり、Unicode（UTF-8等）での統一が一般的になった状況では、エミュレーションの脆弱性は考えにくい
し・か・し・な・が・ら、原理的に安全というのと、実装に問題がなければ安全というのは、安全のレベルとしては全然違います。なので、最近私は、無理に静的プレースホルダ（エミュレーションオフ）を推奨してはいませんが、「理論的にはこうなので、組織あるいはプロジェクトとして、リスクとメリットを考慮して決めてください」という言い方をしています。
とのことです。
また「安全なWebアプリケーションの作り方 p.163」では複文の実行を禁止する設定としてPDO::MYSQL_ATTR_MULTI_STATEMENTS => falseも設定していますが、PHP 5.5.21 および 5.6.5 以降が対象のようです。
PHP: MySQL (PDO) - Manual
「この定数が使えるのは、データベースハンドルを新規作成する際の driver_options 配列内だけであることに注意しましょう。」
PDOに複文実行を禁止するオプションが追加されていた | 徳丸浩の日記
「すなわち、new PDOする際に、driver_options配列に上記を設定すればよい」「PDO::MYSQL_ATTR_MULTI_STATEMENTS が存在する場合のみ、このオプションを指定」
BadTodoに合わせるとこんな感じかと思います。
<?php ～（中略）～ $opt = array(PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_EMULATE_PREPARES => false); if (defined('PDO::MYSQL_ATTR_MULTI_STATEMENTS')) { $opt[PDO::MYSQL_ATTR_MULTI_STATEMENTS] = false; } $dbh = new PDO("mysql:host=$dbhost;dbname=todo;charset=utf8", 'root', 'wasbook', $opt);
さらに上記teratailのQ&A内で「2021/04/22 に開催された Club MySQL #5 ～SQLデータベースのセキュリティ～ の動画で、徳丸さんが『動的プレースホルダを使用することのメリット』に関してユーザ会に質問している動画」のリンクも貼られていて全体を通して興味深い内容でした。
Club MySQL #5 ～SQLデータベースのセキュリティ - YouTube
日が経って再度脆弱性を試すときに修正済みであったことを忘れて「あれ？動かない」となりがちなので忘れないうちに元のコードに戻しておいてください。
後日「Todo検索画面」の方も修正を試します。こちらには「LIKE記述とワイルドカード(%や_)」についての問題もありそうです。
SQL Injection Prevention - OWASP Cheat Sheet Series
SQLインジェクション対策の極意はSQL文を組み立てないことにあり (1/4)|CodeZine（コードジン） 2ページ目以降のストアドプロシージャでのSQLインジェクション対策についても一読を。
次回：BadTodo - 3.10 ブラインドSQLインジェクション (Boolean-Based) 練習 - demandosigno
