PEAK XOOPS - News

(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;
    }

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

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

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

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

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

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

'OR 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();

世界的に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 の開発者に言いたいことは以下の３つです。
XOOPS_TRUST_PATHをアーカイブのhtdocs/の外に出してください。
DocumentRootの外と内の違いについて勉強してください。
Protector V3のインストール方法を繰り返し読んでください。

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

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

先日、gusagiさんより、bulletinHDについてのバグレポートとして、以下のURLを転送してもらいました。
http://ryus.co.jp/modules/d3blog/details.php?bid=107
先に結論から書くと、このエントリはMySQLの基本的なcase sensitiveルールを誤解していることによる、間違ったレポートです。
Quote:

SELECT (snip) WHERE dirname='PICO'

ALTER TABLE (prefix)_modules MODIFY `dirname` varchar(25) binary NOT NULL default '';

$constpref = '_MI_' . strtoupper( $mydirname ) ;