[PHP-users 11844] Re: ＰＨＰバージョンによる相違

[Yasuo Ohgaki]

大垣です。

Osamu Shigematsu wrote:
> 変数汚染バグとして大垣さんはどのようなケースを想定されていますか?
> 私は単純に初期化していない変数に意図しない値を送りつける以外の
> 危険性が見出せません。

非常に単純明解なケース（でも割とよく見られる）

if ($login) { }

など、を想定しています。問題は書き換えられる変数の使われ方と思いま
す。仕組みは単純ですが、コードが長くなると段々と初期化されている
はずの重要な変数が未初期化のまま利用されていても判りづらくなり、
バグが入り込む可能性が段々高くなってきます。

こんなコードと書くのが間違っていますが、前のコードも以下の感じで記
述されたコードがあると一見しただけでは安全かどうか判りません。しか
も、ほとんどのケースで$statusは初期化されているため、E_ALLでも
気が付かない場合もあると思います。

if ($some_condition) {
  $status = get_some_login_status();
   // some code
}
// do something

if ($other_condition) {
  $status = get_other_login_status();
   // some code
}
// do other thing

if ($another_condition) {
  $status = get_another_login_status();
   // some code
}
// do another thing

if ($yet_another_condition) {
  $status = get_another_login_status();
  // some code
}
// do yet another thing

if ($status == 'OK_TO_DO_IMPORTANT_JOB') {
  // valid status is set, so do critical thing
}

PHPのもともとの設計思想はグローバルスコープで処理はほとんど行なわ
ない ＋ 関数・クラススコープからはグローバル変数は見えない、したがっ
てGPC変数をグローバル変数として初期化しても安全かつ便利、と言う事
だったのではないかと思っていますが、register_globals=Onは使い
勝手は悪過ぎです...

他には、普通に関数を使っていれば大丈夫ですが、変数汚染を利用すると
ファイルアップロードアタックを行なってサーバーの任意のファイルを読
みとる事が可能になったり、ファイルの書き換えが行なえる場合もありま
す。

> ただ、出所がわかったところで、hidden で post した値だから、
> 内容を精査しない、というのでは、出所が確認できたところで、
> まったく安全になりえません。

全くその通りです。
hiddenで変数を渡す場合、配列にした値をserialize、圧縮、urlencode
しmessage digest方式で改竄チェックをするsecure_serialize
関数という関数を作ってチェックしています。ずいぶん前に、zend.com
のCode Exchangeにポストしているコードと仕組みは同じです。

# 普通の変数は必要な変数が全て設定されているか、設定されている
# 変数は正規表現、長さ、範囲、etcが制限に合致しているかをカス
# タムフォームクラスでクライアントとサーバーで両方でチェックしています。
# このクラスを使うとフォーム入力、入力値の確認、入力された情報を
# 文字としてクライアントに出力する確認ページの表示、JavaScript
# が使えるクライアントではJavaScriptによるクライアントサイド
# でのチェック、フォーム多重送信制限などを、必要なエラーメッセー
# ジなどの作成、配列で定義するフォーム定義を作成するだけでほぼ
# 自動化できます。
# おかげで、フォーム処理はかなり重ためです。Cモジュールを作って
# 公開しよう思っていますが、滞っています :(

このMLにもポストしました例＆変数汚染以前の問題ですがイメージファ
イルを送信するスクリプトで以下のような処理をしていたPHPアプリが
BugTraqに載っていました。

<?php
$file = '/base/path/to/image/files/'.$name;
header('Content-Type: image/gif');
readfile($file);
?>

http://example.com/image.php?name=abc

丸見えです...
実際にはもう少し長いコードでしたが、要旨はこういった感じでした。

> たとえば、変数の汚染に対処するだけならば、
> 
> function my_proc(){
> 	// globals で引っ張る変数を制限するなど...
> 	// ここに本来の処理を書く
> }
> my_proc();
> 
> これだけで、変数の汚染から随分身を守れると思います。
> 実際のところ、他人が開発したスパゲッティプログラムを改修するときに、
> 変数の上書きを防ぐために用いた手段です。

register_globals=Onでなければならない場合、同じような
関数を書くと思います。
安全なスクリプトにする事はできますが、その後のメンテナンスを
考えるとやはりregister_globals=Onは危険と思います。

>>register_globals=onの危険性はBlackHatなどで何年間
>>に渡って何度も指摘されているにも関わらず、register_globals=on
>>により多くの危険なスクリプトが量産されてきた現実があります。
>>
>># 危険なスクリプトには変数汚染以外の問題もある場合が多い
>># ですが...
> 
> 
> というか、危険なスクリプトは、変数汚染以外にも問題があり、
> 言い換えると、register_globals = on にしても、
> 問題は解決しないだろうと推測します。
> # 変数汚染は少なくとも XSS 脆弱性などよりも気づきやすい穴と思います。

XSS、SQLインジェクション、両方とも場当たり的対処をしていると問題
が発生しやすくなりますね。両方とも関数/クラスを作っています。
関数/クラスを使っている限りは、絶対に脆弱性が発生しないようにコー
ドを書いておくべきと思います。場当たり的な対処も必要（便利）な場合
もありますが、できるだけ控える方が良いと思います。

# pg_insert()/pg_update()/pg_select()/pg_delete()は
# その一部をPostgreSQLモジュールに取り込んだ物です。

magic_quote_gpcも不必要な機能ですね。非効率なだけでなく問題が多
いです。出力する際にはstrip_slashesをしなければならない->コード
が読みづらくなる->SQLインジェクションの温床になる
(magic quote == SQLインジェクションセーフと言う勘違いも含めて）
register_globalsと同じくらい使い勝手が悪くなる機能と思います。

>>車に例えるなら、register_globals=onはブレーキの効きを
>>悪くする変わりに、多少乗り心地を良くできる危険な機能、と言っ
>>た所と思います。
>>
>>ブレーキの効きが悪くても車を安全に運転する事はできますが、
>>効率が良いとは思えません。
> 
> 
> それは違うような。。。

私もそう思っているのですが「register_globals=Onは乗り心地
が良い」と考えている方も多いようなので、こう書いていました。

> 
> 今までのスクリプトとの互換性が損なわれたに過ぎず、
> 新規に開発するならば、むしろ、効率はよいのですから、
> 乗り心地 (= 開発効率) はよくなっているはずです。
> そして、安全性も向上していると思います。

私はOffの方がずいぶん乗り心地が良いと思っているので、機会がある
毎にregister_globals=Offをお勧めしています。

# magic_quotes_gpc=Offも

> つまり、便利で快適な装置なんだけど、旧車につけると、
> 新車を買うくらいコストがかかることがあるかも、見たいな感じでは。

やっつけ仕事で作成したシステムは改修する方がコストが高いという
事になるかも知れません。
しかし、一概にSI/プログラマが悪いという訳でも無いと思います。
無理を承知で納期/コストを削減して発注した場合、発注者の責任も
（もしかしてSIの営業の責任も）大きいと思います。安全/高性能/
保守性が高い成果物を提供するにはそれなりのコスト/納期が必要で
すから... でも現実は... と言う事かも知れません...

--
Yasuo Ohgaki