PEAK XOOPS - News in englishin japanese

Archive | RSS |
  
Poster : GIJOE on 2009-01-23 05:47:03 (18202 reads)

in englishin japanese
(3)で完成したと思っていたDBLayer Trapping anti-SQL-Injectionのロジックですが、これを実際にXOOPS 2.0.16a-JPでテスト運用してみたら、一般設定の更新でSQL Injectionの誤検出が発生しました。(具体的には、picoの共通ヘッダとしてダブルクオーテーションを含む文字列をポストする時)

このSQL対策ロジックは原理上、誤検出も当然あり得るのですが、どう見てもそういうパターンでもないので、疑問に思いながらXOOPSのソースを追ってみたら、かなりビックリするコードを見つけました。

class/database/mysqldatabase.php


    function quoteString($str)
    {
         $str = "'".str_replace('\\"', '"', addslashes($str))."'";
         return $str;
    }

ん? \"を"に書き戻している? なぜ?

ほとんどの文字列表現実装系で、シングルクオーテーションの内側でも"はエスケープの対象になると思いますし、実際、MySQLの文字列表現もそうなってます。なぜXOOPSのquoteString()がなんでこんな怪しいことをしているのか理解に苦しみますが、結果的に文字列のパースに間違いが出たこともなかったので、誰も突っ込まなかっただけなのでしょう。

いずれにせよ、このメソッドの正解は以下のコードです。

    function quoteString($str)
    {
         $str = "'".str_replace('\\"', '"', addslashes($str))."'";
         $str = "'".mysql_real_escape_string($str)."'";
         return $str;
    }

MySQL用のDBレイヤーなのですから、mysql関数が堂々と使えますし、逆に、MySQLのクライアントエンコーディングがSJISやBIG5にならない、なんて判断はこのメソッドには出来ないわけですから。

ふと気になって、ImpressCMSと本家版2.3.2を調べてみたら、どちらもちゃんとmysql_real_escape_string()を使う実装になってました。おそらくその変更タイミングは、本家版の2.0.14〜17あたりでしょうか。

つまり、XOOPS Cubeプロジェクトだけが取り残されている状況だったんですね

ただ、いずれのコアを使う場合であっても、今回と同様の誤検出は考えられるので、Protector-3.3.1では、以下のようなロジックを追加導入しました。

「'や"を含んだリクエストが、SQL内にエスケープされていない状態で含まれていたとしても、そのリクエスト全体が、SQLの文字列内に収まっている(=クオーティングを破っていない)ならSQL Injectionではない」

これで誤検出そのものも減りそうです。

0 comments

Poster : GIJOE on 2009-01-16 05:07:17 (15858 reads)

in englishin japanese
DBレイヤーのオーバーライドの準備ができたとしても、リクエストとすべてのSQLを比較するのは、それなりにコストが高い手法であることを自覚する必要があります。

そこで、まずはDBレイヤーをオーバーライドする頻度を下げる努力をします。

ほとんどのリクエストには「攻撃となりえる」文字列は最初から含まれていないのですから、その時には元のDBレイヤーをそのまま使います。
あくまで、特定の文字列が含まれている時だけ、DBレイヤーをオーバーライドすることで、速度と安全性を両立できるでしょう。

もちろんその「特定の文字列」の判断こそが難しいのですが、こんなルールにしてみました。


/(information_schema|select|'|")/i

ここでは、引用符を壊す可能性のある'および"、そして、他のテーブルを参照する可能性のあるselectのみを検出しています。
unionは単独では意味を持たないのでここでは考慮しません。
また、コメント開始記号は誤認識が多い上(特に $_SERVER['HTTP_ACCEPT'] に*/*が含まれることが多い)、引用符を壊してからでないと攻撃としてもほぼ意味がないので、特定の文字列には含めません。


さて、SELECTや'や"を含んだ、「攻撃となりえる」リクエストを検出して、DBレイヤーのquery()メソッドをオーバーライドしたとしましょう。

次は、query()メソッドのなかで、SQL Injection攻撃の可能性があるリクエストと、実際に発行しようとしているSQLを比較します。

SQL Injection脆弱となるパターンには、大きく分けて2つあります。

(1) リクエストがクオーテーションの内側に収まるが、エスケープし忘れたもの
例)
SELECT ... FROM `table` WHERE `column`='(エスケープし忘れた文字列)'

(2) リクエストが最初からクオーテーションの外側にあるもの
例)
SELECT ... FROM `table` WHERE `integer_column`=(リクエスト)
SELECT ... FROM `table` WHERE ... ORDER BY (リクエスト)

ほぼ確実に防げるのは(1)のパターンです。

具体的には、リクエストの中で、シングルクオーテーションやダブルクオーテーションを含む文字列を記憶しておき、その文字列がそのままSQLに含まれるようなら、SQL Injectionだと判定してしまいます。というのも、まともなPOST文字列などであれば、クオーテーションの内側に収まっているはずなので、エスケープされていなければならないからです。

'や"を含む文字列をリクエストされた時に、たまたまリクエスト非依存のSQLと、偶然に一致してしまって、SQL Injection対策が効きすぎてしまう恐れはありますが、これはチェックすべきリクエスト文字列の最低字数を長くすることで可能性を下げることができます。さじ加減が難しいところではあるのですが、ギリギリ6文字未満の文字列は見逃す、くらいのバランスであれば、悪意あるSQL Injectionとしては有効な攻撃にならず、かつ、誤検出もほとんどでないでしょう。

このパターンでの最短と思われる有効な攻撃文字列はこうですから。
(もっと短いのがあったら教えてください)

'OR 1#


逆に、(2)のミスをカバーしきるのは、かなり苦しいと言わざるを得ません。(2)のようなSQL Injection脆弱性がある時点で、そのtableのデータが読み書き自在とされるのは諦めるべきでしょう。

ただ、SQLとリクエストとの比較ロジックで頑張れば、他のテーブルへの波及はギリギリ防げると思います。

具体的には以下の2つの方法の併用です。

(A) SQL内にコメントがある時点でアウトとする
(B) SQLから文字列部分を取り除いたものの中に、SELECT等を含むリクエスト文字列が存在すればアウトとする

(A)はやりすぎな気もするのですが、現実にコメント付きのクエリを発行することはまずありませんし、怪しいリクエストが存在した時しかこのロジックは効かないので、ほぼ問題にならないだろう、と判断してます。

(B)がこのロジックの肝です。サブクエリにしてもUNIONにしても、基本はSELECTをどう埋め込むかです。そのSELECTを含んだリクエスト文字列が、クオーテーションの内側にない、という時点でアウトと判断できれば、他のテーブルに対するほとんどの攻撃を防げるでしょう。(もちろん、テーブル名一覧をinformation_schemaから取得する攻撃も)

このあたりの具体的な実装は、Protector-3.3 のProtectorMySQLDatabase.phpを確認してもらうのが良いと思います。

0 comments

Poster : GIJOE on 2009-01-15 16:12:57 (15427 reads)

in englishin japanese
あっという間に1週間経ってしまいましたが、「リクエスト層とDB層の両方を比較して、SQL Injectionを防ぐ方法」の続きです。

PHP汎用の話題であれば、何らかのフレームワークにこれを組み込む、という話になるのでしょうが、今どきのフレームワークはORマッパーを使うものが多く、クエリを直接発行することもないでしょうから、例によってXOOPS用モジュールであるProtectorにどう組み込むか、という話になります。

まず、DBレイヤーをフックする必要がありますが、XOOPSの場合、ここが非常に硬直化していて、後からDBインスタンスを乗っ取ることはほぼ不可能です。つまり、いずれのXOOPSコアについても、データベースファクトリクラスを書き換える必要があります。

フック方法もいろいろ考えたのですが、絶対にリクエスト依存にならない、という理由から、定数を利用することにしました。最初に、XOOPS_DB_ALTERNATIVEという定数が宣言されていれば、そのクラスがDBインスタンスとして用意されます。

class/database/databasefactory.php


			require_once $file;
			/* patch from */
			if ( defined('XOOPS_DB_ALTERNATIVE') && class_exists( XOOPS_DB_ALTERNATIVE ) ) {
				$class = XOOPS_DB_ALTERNATIVE ;
			} else /* patch to */if (!defined('XOOPS_DB_PROXY')) {
				$class = 'Xoops'.ucfirst(XOOPS_DB_TYPE).'DatabaseSafe';
			} else {
				$class = 'Xoops'.ucfirst(XOOPS_DB_TYPE).'DatabaseProxy';
			}
			$instance =& new $class();

これはこれで融通の効かない仕組みではあるのですが、DBインスタンスを直に書き換えるメソッドを追加する、なんてのも危険な気がしたので、各コアチームが採用しやすい形を最優先に考えた結果、こうなりました。

Protector-3.30にはそのパッチも含めたので、ぜひとも採用の検討、お願いします。>minahitoさん・marcanさん・phpppさん

次回はオーバーライドする条件および、比較ロジックについて書きます。


Poster : GIJOE on 2009-01-09 13:09:35 (19595 reads)

in englishin japanese
世界的にXOOPSの本家とは xoops.org ってことになり、本流は2.3系列って事になっているようです。

そこの最新版である xoops-2.3.2b というアーカイブをダウンロードしてみてあきれました。

htdocs/ の中に、xoops_lib というフォルダがあって、その中に、XOOPS_TRUST_PATHに置かれるべきファイルが詰まっているのですから。
しかも.htaccessすらなし。

思わず目が点になりました。

mamba氏が、XOOPS_TRUST_PATH内のファイルにLFIがある、という不思議なレポートをしてきて、その意味が理解できなかったのですが、アーカイブ構造を見て納得しました。
つまり、彼らはXOOPS_TRUST_PATHの意味を一つも理解していないのです。
XOOPS_TRUST_PATHを、xoops_lib なんて名前に勝手に変更しているのも、彼らがXOOPS_TRUST_PATHの意味を理解していないことの証拠の一つでしょう。

mamba氏は「Protectorにセキュリティホール(LFI)があるから独自にfixをした」、とか言ってましたが、DocumentRootの外にあるのが前提となっているファイル群に場当たり的なパッチを当ててもまったくの無意味です。

案の定、今度はRFIとしてレポートが上がったそうです。
http://www.milw0rm.com/exploits/7705

これはProtectorの穴ではなく、XOOPS-2.3.2の穴であると正確に理解してください。

とにかく、phppp はじめ xoops.org の開発者に言いたいことは以下の3つです。
XOOPS_TRUST_PATHをアーカイブのhtdocs/の外に出してください。
DocumentRootの外と内の違いについて勉強してください。
Protector V3のインストール方法を繰り返し読んでください。

それが出来ないのなら、同梱なんてしないでください。

誤ったインストール方法をデフォルトで用意しておいて、Protectorにセキュリティホールがあるなんて言ってくるxoops.orgの神経が信じられませんし、それが原因で第三者にProtectorの存在意義を問われてしまうのもこれまた心外です。


Poster : GIJOE on 2009-01-07 04:37:49 (17733 reads)

in englishin japanese
XOOPSに限らず、いろいろなPHPアプリケーションで、今でもちょくちょくSQL Injection脆弱性が出てきます。それもケアレスミスです。

もちろん、ケアレスミスは人力では根絶不可能であり、SQLを文字列として生成すること自体をやめて、パラメータ方式で呼び出すべきだ、という方が本質的な議論でしょう。

ただ、オープンソースの世界では、自分の考えを他者に押しつけることはできませんし、SQL Injectionの可能性ある「誰か」の書いたコードを一切利用しない、というのも現実的ではありません。

事実として、SQL InjectionはXSSとは比べものにならないくらい恐い攻撃です。もし私がオープンソースアプリケーションで構築された特定のサイトを攻撃するとしたら、最初に狙うのはSQL Injectionでしょう。なぜならXSSやCSRF、セッション関連の攻撃と違い、即座に直接攻撃可能だからです。(さすがにコマンドインジェクションとかRFIはない、という前提で)

そういうわけで、Protectorでは、SQL Injection対策っぽいものも入っているのですが、私自身、あまり役に立っている気がしません。攻撃者がProtectorの対策コードを読めば、明らかに抜け道だらけです。(本当に役に立ったとすれば「最後にidのつくリクエスト変数名について強制的にintval()をかける」くらいでしょうが、これも副作用が大きすぎます)

強力なSQL Injection対策ができない理由は簡単で、本当に必要なリクエストかどうか区別がつかないからです。「UNIONをUNI-ONにする」なんてのも、本当はあってはならないことです。UNIONという単語を使ったまともな投稿が勝手に書き換わってしまいます。

ただ、入口であるリクエスト層だけで判断している以上は、ここが限界となりますが、大きな傘anti-XSSシステムと同様、入口と出口の両方を押さえれば、かなり有力な手段となることにふと気がつきました。これはいけそうです。

anti-XSSでは、出口はobフィルターでした。XSSになる可能性があるリクエストがある時のみob_start()をかけて、出力にリクエストがそのまま含まれるようならXSSとして停止する。

これをanti-SQL-Injectionにあてはめると、出口は当然クエリ関数(メソッド)となります。SQL Injectionになる可能性があるリクエストがある時のみ、DBレイヤーを乗っ取って、クエリ本文(クオーテーション外)にリクエストが含まれるようならSQL Injectionとして停止する…

…とまあ、文章として書くのは簡単なのですが、SQL Injection用の出口処理は結構面倒です。SQLをちゃんとパースする必要がありますから。

また、この仕掛けをXOOPS用Protectorとして採用する場合、もう一つ問題があります。それは、現状のDBレイヤーは、途中で乗っ取ることが不可能、ということです。この点は、コア側に対応してもらう必要があるため、別エントリにしたいと思います。


ちなみに、今回私が考え出した(と思っていた)方法ですが、元ネタがあったのを思い出しました。
「本質的なSQL Injection対策のためにはDBレイヤーを乗っ取るしかない」
と、もう4年も前にJM2さんが指摘していました。流石としか言いようがありません

0 comments

« 1 (2) 3 4 5 ... 55 »
Login
Username or e-mail:

Password:

Remember Me

Lost Password?

Register now!