月別: 2007年8月

遅ればせながらCore Graspのパッチを読みました。超ななめ読みなので勘違いしているかも知れません。間違っていたら教えてください。

一番興味があったのはSQLインジェクションの自動検出はどうなっているのかです。以下の関数がSQLインジェクションチェックに利用されています。

PHP

+int grasp_check_query(zval *z) +{ + char *c,*s; + int i,j,l; + char q; + + if (grasp_isfull_p(z)) return 0; + if (!grasp_isptr_p(z)) return 1; + if (z->type != IS_STRING && z->type != IS_CONSTANT) return 1; + + + l = z->value.str.len; + c = z->value.str.val; + s = z->secmark; + + + for(i = 0; i<l; i++) + { + if (s[i] && (c[i]=='-')) + { + i++; + if(!(c[i] < '0' || c[i] > '9')) + { + for(i;i<l;i++) + { + if (c[i] < '0' || c[i] > '9') + { + if (!s[i]) + break; + else + return 0; + } + } + } + else + { + return 0; + } + if(i == l) return 1; + } + + + if (s[i] && (c[i] < '0' || c[i] > '9')) return 0; + + if (c[i] == '¥¥') + { + i++; + if (s[i]) return 0; + } else + if (c[i] == '¥'' || c[i] == '¥"' || c[i] == '`') + { + q = c[i]; + for(i++; i<l; i++) + { + if (c[i] == '¥¥') + i++; else + if (c[i] == q) + break; + } + if (i<l && s[i]) return 0; + } + } + return 1; +}

0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65

+int grasp_check_query(zval *z) +{ +    char *c,*s; +    int i,j,l; +    char q; + +    if (grasp_isfull_p(z)) return 0; +    if (!grasp_isptr_p(z)) return 1; +    if (z->type != IS_STRING && z->type != IS_CONSTANT) return 1; +     +     +    l = z->value.str.len; +    c = z->value.str.val; +    s = z->secmark; + + +    for(i = 0; i<l; i++) +    { +        if (s[i] && (c[i]=='-'))             +        { +            i++;                     +            if(!(c[i] < '0' || c[i] > '9')) +            { +                for(i;i<l;i++)             +                { +                    if (c[i] < '0' || c[i] > '9') +                    { +                        if (!s[i]) +                            break;     +                        else +                            return 0; +                    } +                } +            } +            else +            { +                return 0; +            } +            if(i == l) return 1; +        } +         + +        if (s[i] && (c[i] < '0' || c[i] > '9')) return 0; +         +        if (c[i] == '¥¥') +        { +            i++; +            if (s[i]) return 0; +        } else +        if (c[i] == '¥'' || c[i] == '¥"' || c[i] == '`') +        { +            q = c[i]; +            for(i++; i<l; i++) +            { +                if (c[i] == '¥¥') +                    i++; else +                if (c[i] == q) +                    break; +            } +            if (i<l && s[i]) return 0; +        } +    } +    return 1; +}

どうも文字列の各文字が安全かチェックして安全でない場合はエラーにしているようです。単純に「この文字列（ZVAL）は関数で処理済みで安全」とマークしている訳でないようです。

ではmysql_real_escape_stringとかを拡張して安全マークを追加しているのかな?と思ったらしていないようです。代わりにaddslashesが拡張されていました。

PHP

PHP_FUNCTION(addslashes) { zval **str; +#ifdef HAVE_GRASP + char *res, *secm; +#endif if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &str) == FAILURE) { WRONG_PARAM_COUNT; @@ -2884,11 +3266,23 @@ PHP_FUNCTION(addslashes) if (Z_STRLEN_PP(str) == 0) { RETURN_EMPTY_STRING(); } +#ifdef HAVE_GRASP + grasp_normalize(*str); + res = 0; + secm = 0; + //grasp_setptr_p(return_value, zend_get_parameters_secmark()); + res = php_addslashes_secmark(Z_STRVAL_PP(str), + Z_STRLEN_PP(str), + &Z_STRLEN_P(return_value), 0, &secm, Z_SECMARK_PP(str) + TSRMLS_CC); + RETURN_STRING_SECMARK(res, 0, secm); +#else RETURN_STRING(php_addslashes(Z_STRVAL_PP(str), Z_STRLEN_PP(str), &Z_STRLEN_P(return_value), 0 TSRMLS_CC), 0); +#endif }

0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32

PHP_FUNCTION(addslashes) {     zval **str; +#ifdef HAVE_GRASP +    char *res, *secm;     +#endif     if (ZEND_NUM_ARGS() != 1 || zend_get_parameters_ex(1, &str) == FAILURE) {         WRONG_PARAM_COUNT; @@ -2884,11 +3266,23 @@ PHP_FUNCTION(addslashes)     if (Z_STRLEN_PP(str) == 0) {         RETURN_EMPTY_STRING();     } +#ifdef HAVE_GRASP +    grasp_normalize(*str); +    res = 0; +    secm = 0; +    //grasp_setptr_p(return_value, zend_get_parameters_secmark()); +    res = php_addslashes_secmark(Z_STRVAL_PP(str), +                                 Z_STRLEN_PP(str), +                                 &Z_STRLEN_P(return_value), 0, &secm, Z_SECMARK_PP(str) +                                 TSRMLS_CC); +    RETURN_STRING_SECMARK(res, 0, secm); +#else     RETURN_STRING(php_addslashes(Z_STRVAL_PP(str),                                  Z_STRLEN_PP(str),                                  &Z_STRLEN_P(return_value), 0                                  TSRMLS_CC), 0); +#endif }

マルチバイト環境ではaddslashesによる文字列エスケープは厳禁な（SQLインジェクションに脆弱になる場合がある）ので実装的には不十分です。

GraspはPHP用のtaintモードとして紹介されていますが、Perl, Rubyなどの一般的にtaintモード呼ばれている機能と多少異なります。

http://www.cs.virginia.edu/~evans/pubs/infosec05.html

に記載されているようなtaintモードになっているようです。
# この論文もななめ読み

具体的には

$sql = “SELECT * FROM table WHERE id = “. $id;
mysql_query($sql);

の様なコードでも$idが危険な文字列かつ攻撃コードを含んでいれば、エラーになります。つまり、$sql文字列中の$id部分が安全かどうかチェックできるようになっています。

例えば、

if (ereg(“[0-9]+”, $_GET[‘id’]) {
$id = $_GET[‘id’];
}

の様にプログラマのミスにより$idが危険な変数になっていても攻撃が回避できます。

完成度が高くなれば通常のtaintモードとは比べ物にならないくらい安全性の向上が期待できると思います。しかし、パッチを超ななめ読みした感じでは実用的になるまでもうしばらく時間が必要な気がしましたが、順調に開発が進めばかなり便利な機能になると思います。
# 現状の実装だど文字エンコーディングがSJIS等の場合はSQLインジェクション
# と誤検出すると思います。addslashesなので元々SJIS等の事などまったく考慮
# されていなくて当たり前です。マルチバイト文字の事は考えてないようですが
# UTF-8ならそれなりに使えそうです。

SQLインジェクションは対策が簡単ですが、XSSは適材適所で対応が分かれるので精度の高い対策は難しいです。しかし、LDAP/XPATH/XQuery等のインジェクション攻撃は基本的にSQLインジェクションと同じレベルで対応可能です。半年後くらいにまたパッチを見てみよう。

「なぜPHPアプリケーションにセキュリティホールが多いのか？」をテーマにした技術評論社のブログ風の記事を書いています。

一番新しい

http://gihyo.jp/dev/serial/01/php-security/0008

ですが、自分で防いだつもりのセキュリティホールは実は防いだ事になっていなかった、アプリケーションが多く在りました。というより今でも作り続けられています。原因は「サニタイズ」による脆弱性の回避であるものが多いです。なので「サニタイズ」ではなく「バリデーション」と「適切なエスケープ」によって対策すべきです、と書きたかったのですが「エスケープ」の部分を書いてないですね。バリデーションの時に説明するつもりですが、確かに尻切れとんぼのような感じです。

編集の方からこんな感じなのですがどうなんでしょう？メールが来ていました。

http://b.hatena.ne.jp/entry/http%3A//gihyo.jp/dev/serial/01/php-security/0008

と言う感じで何やら否定的なコメントが多いようです。

最初に

今回は熟練したWebアプリ開発者なら常識のクロスサイトスクリプティング対策の落とし穴を紹介します。

と書いているのですがどうも「熟練のWebアプリ開発者なら常識」としている前提が伝わっていなかった？のか記事自体に問題がある？のか理由がいまいち分かりません。

ブックマークしている方を見るとセキュリティ系で有名な方もいます。初心者向けのブログなのであまり一般受けしないエントリはこのブログの方が多はずです。（と言ってもこちらにもあまり役に立つことはあまり書いてませんが）いまいち理由がよくわからないのでもし知っている方教えてください。

ご存知の方も多いと思いますがPHP 5.2.4のリリースが近いです。しばらく前にRC1が出ています。

http://qa.php.net/

どのWebアプリケーションもですがインフラとなるソフトウェアのバージョンアップをどう行うか定めておかなければなりません。オープンソースの場合、いつでもソースコードのリポジトリから開発版が利用できるので開発版を利用してアプリケーションの動作に支障が無いかテストできます。

しかし、普通は開発版で常に動作テストは行えません。次のバージョンがリリースされる前にしばらくの間RC版（Release Candidate版）がテスト用に配布されます。RC版の間であればクラッシュバグや意図的でない動作仕様の違いなどをバグレポートとして提出すればリリース前に修正される可能性が高くなります。パッチを付けて送付すれば適用される可能性も高くなります。RCで動作テストを十分行っていればリリースされた場合にセキュリティフィックスが含まれいても慌てることはありません。

大規模にシステムを運用している会社であればRC版からの動作テストのメリットは非常に多いです。RC版からの動作テスト&フィードバックを強くお勧めします。