2015年2月23日月曜日

PHPのmb_ereg関数群は不正な文字エンコーディングをチェックしない

PHPのbasename関数には、マルチバイトに対応していないという誤解(実際にはロケールの設定をすればマルチバイトでも使える)があったり、不正な文字エンコーディングをチェックしないという課題があったりで、イマイチだなーと思っている方も多いと思います。
そういう方々が、preg_replace(u修飾子つき)やmb_ereg_replaceを用いて代替関数を作成している解説も見かけますが、それではこれら正規表現関数は不正な文字エンコーディングをチェックしているのだろうかという疑問が生じます。
ざっと調べたところ、以下の様な状況のようです。
  • preg_replace : 不正な文字エンコーディングをチェックしている
  • mb_ereg_replcae : 不正な文字エンコーディングをチェックしていない
ここでは、mb_ereg_replaceが不正な文字エンコーディングをチェックしない状況と、その影響について報告します。

不正な文字エンコーディングとは

マルチバイトの文字を表現するエンコーディング(Shift_JIS、EUC-JP、UTF-8など)には、バイト列の並びのルールが決まっています。例えば、Shift_JISの2バイト目としてありえるバイト値とあり得ないバイト値があります。UTF-8の場合は、先頭バイトで文字のバイト数が決まり、2バイト目以降に使えるバイトは 0x80 ~ 0xBF と決まっています(参考)。
これらの決まりに従わないバイト列は、「不正な文字エンコーディング」ということになります。また、UTF-8には「非最短形式」というものがあり、禁止されているので、これも不正な文字エンコーディングの一種です。

mb_ereg_replaceは不正な文字エンコーディングをどう扱うか?

ここでは、UTF-8の場合を例として、mb_ereg_replaceが不正な文字エンコーディングのデータをどのように処理するかを見てみます。
まず、不正なデータとして以下を例に取ります。
$a = "\xFC../..";  // 6バイト
このデータは以下の意味でUTF-8として不正です。
  • 0xFCは、かつてUTF-8の6バイト形式として定義されていたが、現在は5バイト以上の形式は禁止されている
  • 2バイト目以降が 0x80~0xBF の範囲にない
PoCは以下の通りです。
<?php
  mb_regex_encoding('UTF-8');
  $a = "\xFC../..";
  $s = mb_ereg_replace('[\./]', '', $a);  // . と / をすべて取り除く
  echo bin2hex($s) . "\n";
結果は以下の通り。
fc2e2e2f2e2e
先頭の0xFCの影響を受けて、2バイト目以降の . や / は取り除かれていません。ここから以下のことが分かります。
  • mb_ereg_replaceはUTF-8の6バイト形式を許容している
  • mb_ereg_replaceはUTF-8の2バイト目以降のバイト値の範囲をチェックしていない
この結果は私には衝撃的だったのですが、ディレクトリトラバーサル脆弱となる現実的なシナリオは作れませんでした。腕自慢の方は挑戦してみてください。

脆弱となる例

ディレクトリトラバーサルの例は作れませんでしたので、XSSならどうだろうと思い、ちょっと人工的ですが、不正な文字エンコーディングによるXSS脆弱性の例を考えてみました。以下のスクリプトはmb_ereg_replaceを使った「手作りの」HTMLエスケープ関数を使っています。
<?php
  header('Content-Type: text/html; charset=UTF-8');
  function mb_htmlescape($s) {
    mb_regex_encoding('UTF-8');
    $s = mb_ereg_replace('&', '&amp;', $s);
    $s = mb_ereg_replace('<', '&lt;', $s);
    $s = mb_ereg_replace('>', '&gt;', $s);
    $s = mb_ereg_replace('"', '&quot;', $s);
    return $s;
  }
?>
<html><body>
<?php echo mb_htmlescape($_GET[x]); ?>
</body></html>
このスクリプトに対して、x=<script>alert(1);</script> というクエリ文字列を与えると以下のように正しくエスケープ処理が行われています。


しかし、以下のクエリ文字列だと、JavaScriptが起動されてしまいます。
x=%C2<script+%C2>alert(1);//%C2</script+%C2>

%C2はUTF-8の2バイト形式の1バイト目になるバイト値です。これを■で表現すると、入力文字列は下記の通りです。
■<script ■>alert(1);//■</script ■>
そして、この文字列は、先の手作りエスケープ関数を素通りしてそのままブラウザに送られます。
ブラウザ側では、この■<等が「不正なUTF-8文字」と認識して、■と < という別々の文字として扱われます。この「不正な文字に対する扱いの差異」が脆弱性の原因です。

対策

以下の対策を推奨します。
  • エスケープ関数等セキュリティ目的の処理は極力自作しない(htmlspecialchars関数では上記の問題は起きない)
  • 各入力値についてmb_check_encoding関数により文字エンコーディングの妥当性チェックを行う

まとめ

PHPのhtmlspecialcharsはかつていけてなかった(参考)とか、basename関数は今もいけてないなどの理由でこれらの関数を自作したくなる衝動にかられる場合がありますが、中途半端に自作するとかえって危険になる場合があります。PHPの主要な関数群は色々ディスられながら改良され、安全になってきた歴史がありますので、よほど明確な理由がない限りはPHPで提供されているものを使うほうがよいと考えます。
また、PHPが提供する関数群には文字エンコーディングのチェックを厳密に行うもの(mb_check_encoding、htmlspecialchars等)と、チェックをあまりしないもの(mb_strlen、basename、mb_ereg等)がありますので、入力値のバリデーション時にmb_check_encodingで文字エンコーディングの妥当性を確認しておくとよいでしょう。

2015年2月13日金曜日

PHPのbasename関数は不正な文字エンコーディングをチェックしない

昨日のエントリにて、PHPのbasename関数はマルチバイト文字を扱えることを説明しましたが、このブログの読者であれば、きっとbasename関数は不正な文字エンコーディングについてどの程度チェックするのかという疑問が生じたことでしょう(きっぱり)。実はbasename自体は、不正な文字エンコーディングをチェックせず、垂れ流してしまいます。その理由をbasenameのソースコードで確認してみましょう。以下は、basename関数の実装の一部です。
// ext/standard/string.c
// php_basenmae()
while (cnt > 0) {
  inc_len = (*c == '\0' ? 1: php_mblen(c, cnt));

  switch (inc_len) {
    case -2:
    case -1:
      inc_len = 1;
      php_ignore_value(php_mblen(NULL, 0));
      break;
    case 0:
      goto quit_loop;
php_mblen関数はmblen(3)のラッパーです。mblen関数は文字列の先頭文字のバイト数を返す関数で、先頭の文字が不正なエンコーディングの場合 -1 を返します。上記のソースでは、mblenが-1を返した場合は、inc_len=1として正常な1バイト文字と見なして処理を継続しています。
一方以下は、シェル呼び出しのエスケープを行うescapeshellarg関数の実装ですが…
// ext/standard/exec.c
// php_escape_shell_arg()
for (x = 0; x < l; x++) {
  int mb_len = php_mblen(str + x, (l - x));

  /* skip non-valid multibyte characters */
  if (mb_len < 0) {
    continue;
  } else if (mb_len > 1) {
    memcpy(cmd + y, str + x, mb_len);
    y += mb_len;
    x += mb_len - 1;
    continue;
  }
  switch (str[x]) {
    // 文字に応じた処理
コメントにあるように、不正な多バイト文字はスキップする、すなわち除去(フィルタリング)されます。

同じmblen関数を使っていても、basename関数とescapeshellarg関数では、不正な文字エンコーディングに対する対処方法が違っています。ともかく、basename関数は、不正な文字エンコーディングをエラーとせず、結果の中に含めてしまいます。

不正な文字エンコーディングの影響の考察(Windowsの場合)

basename関数が不正な文字エンコーディングのチェックをしていないことによるセキュリティ上の影響はないでしょうか。具体的に確認するために、まずWindowsの場合について検討します。すなわち、ファイル名が Shift_JIS でエンコーディングされているとします。
ディレクトリトラバーサル攻撃の攻撃パターンとしては、絶対パスによるものと相対パスによるものがありますが、絶対パスの場合ファイル名の冒頭に / や \ が来る必要があり、これは「不正な文字エンコーディング」にはなりえません。相対パスの方は、 ../ などシングルバイトの文字が連続して続く必要がありますが、/や\が単独の場合は単に除去され、その前にマルチバイト文字の先行バイトがある場合でも、前述の理由から先行バイトは除去されません(..■/ の形になる)。.その後 / までが除去される可能性が高いですが、仮に除去されない状況でも、..■/ と余計な文字がはさまっているため、攻撃パターンを形成しないと思われます。

不正な文字エンコーディングの影響の考察(Linuxの場合)

次にLinuxの場合について考えます。文字エンコーディングは UTF-8 とします。この場合、basename関数はUTF-8の冗長表現を通してしまいます。
これを検証するためのスクリプトを以下に示します。\xC0\xAF は / をUTF-8の2バイト表現にしたものです。
<?php
  setlocale(LC_CTYPE, 'ja_JP.UTF-8');
  echo bin2hex(basename("..\xC0\xAFaaa")), PHP_EOL;
出力は下記となります。c0afがそのまま出力されていることがわかります。
2e2ec0af616161
UTF-8の冗長表現が許可されるというと、NimdaワームやTomcatの脆弱性CVE-2008-2938を思い出す人も多いと思います。「それは問題ではないか」と思うところですが、現実には問題になるケースはほとんどないと考えられます。

その理由は下記のとおりです。
  • Linuxで使われるファイルシステムでは、\xC0\xAF等はそのままのバイト列としてファイル名に使われ、ディレクトリ区切子とは認識されない
そこで次の可能性は、UTF-8の冗長表現表現としてbasenameをパスした文字列が、その後文字エンコーディング変換されてシングルバイトの / に変換されることですが、
  • そもそもbasenameの後に文字エンコーディング変換をすることはよろしくない(参考
  • PHPで文字エンコーディング変換に使用される mb_convert_encodingとiconvはどちらもUTF-8の冗長表現をエラーにするかフィルタリングするので、攻撃文字列は形成されない
ということで、basename関数が冗長なUTF-8エンコーディングを許容しても、実害が出るケースはほとんどないと考えられます。実害があるとすると、独自実装の脆弱性のある文字エンコーディング変換機能を利用している場合ですが、その場合でも文字エンコーディング変換後にbasename関数を通すという正しい手順を踏んでいれば、問題は顕在化しません。

緩和策

basename関数は不正な文字エンコーディングを許容することが分かりましたが、これによる実害はほとんどなさそうです。ただし、外部から与えられたファイル名で新規にファイルを作成する場合は、変なファイル名のファイルができてしまいます。
いずれにせよ、以下をアプリケーションの仕様として決めておくとよいでしょう(再掲)。
  • ファイル名に用いる文字の種類
  • ファイル名を表現する文字エンコーディング
  • ファイル名の長さの最小値・最大値
そして、以下を推奨します。
  • 文字エンコーディングの変換はbasenameを通す前に行うこと
  • basename関数を呼ぶ前にlocaleを設定すること
  • ファイル名の仕様を決める
  • ファイル名が(文字エンコーディングを含め)仕様を満たすかどうかバリデーションにより確認する

まとめ

PHPのbasename関数が不正な文字エンコーディングを許容してしまうことを説明しました。この問題は一応bug#68773として報告済みですが、報告から1ヶ月以上たってもアサインもされていませんので、少なくともすぐに修正される可能性は薄そうです。幸い実害もあまりなさそうですが、念のためバリデーションにより文字エンコーディングのチェックをしておくと安心です。
PHPの文字列は単なるバイト列ですので、一般論として、アプリケーションの開始時に文字エンコーディングのチェックをしておくことにより、不正な文字エンコーディングの文字を弾いておくことをお勧めします。アプリケーションの前提条件を満たしていない入力を予め除外しておくことでアプリケーションの安定動作のために寄与します。
また、basenameの現在の実装は少々いただけないと考えます。せっかくmblenが不正な文字エンコーディングをチェックして -1 を返しているのに、そのエラーを「なかったことに」しているからです。一方、シェルのエスケープを行うescapeshellargの方は不正な文字エンコーディングをフィルタリングしているわけで、同じPHPの中で一貫性のない挙動というのも(PHPらしいといえばそれまでですが)よくないように感じました。

2015年2月12日木曜日

PHPのbasename関数でマルチバイトのファイル名を用いる場合の注意

まずは以下のサンプルをご覧ください。サーバーはWindowsで、内部・外部の文字エンコーディングはUTF-8です。UTF-8のファイル名を外部から受け取り、Windowsなのでファイル名をShift_JISに変換してファイルを読み込んでいます。basename関数を通すことにより、ディレクトリトラバーサル対策を施しています。
<?php
  header('Content-Type: text/plain; charset=UTF-8');

  $file_utf8 = basename($_GET['file']);
  $file_sjis = mb_convert_encoding($file_utf8, 'cp932', 'UTF-8');
  $path = './data/' . $file_sjis;
  var_dump($path);
  readfile($path);

しかし、ディレクトリトラバーサル対策は十分でなく、このスクリプトには脆弱性があります。下図は、ディレクトリトラバーサル攻撃により、このスクリプトの中身を読み出しているところです。


以下、このスクリプトの問題点、さらにはbasename関数を用いる際の注意点について説明します。

basename関数はマルチバイト対応していないという誤解

ネットの記事を見ていますと、basename関数はマルチバイト対応していないという主張をよく見かけます。例えば、下記の記事。
basename関数はパスからディレクトリ情報を削除してベース名(「test.txt」など)を取得するための関数ですが、PHP5(使用バージョン:PHP 5.3.1)ではパスに日本語が含まれていると失敗します。

▼失敗するbasename関数
<?php
$a = "/dir/テスト.txt";
echo basename($a);
?>
ファイル名などに日本語が含まれるパスでbasename関数が失敗するバグより引用
このスクリプトをWindowsサーバー上で動かすと、確かに下図のようにファイル名が化けます。下図ではファイル名の16進数表記を参考のためつけています。「テ」の先頭1バイト0x83が欠落しています。



localeに注意

しかし、このスクリプトはbasename関数の使い方に問題があります。basenameのマニュアルには以下の注意書きがあります。
注意:
basename() はロケールに依存します。 マルチバイト文字を含むパスで正しい結果を得るには、それと一致するロケールを setlocale() で設定しておかなければなりません。
これに従い、先のスクリプトを修正してみます。
<?php
$a = "/dir/テスト.txt";
setlocale(LC_CTYPE, 'Japanese_Japan.932'); // 追加
echo basename($a);
こうすると、下図のように正しい結果が得られます。マルチバイト環境でbasename関数を利用するにはlocaleの設定が必要であることが分かります。


冒頭の脆弱なスクリプトへの攻撃法

ここで、冒頭の脆弱なサンプルがなぜ問題かを説明します。このスクリプトにもlocaleの指定がありませんが、それが根本原因ではありません。
まず、攻撃に用いる文字列を示します。以下のクエリ文字列により攻撃が可能です。
..%C2%A5vulbasename.php
%C2%A5はUnicodeの円記号(U+00A5)のUTF-8表記をパーセントエンコードしたものです。これをデコードすると以下の通りです。
..¥vulbasename.php
「¥」は前述のとおり円記号U+00A5です。さらに、これをcp932に変換することになりますが、この際に「¥」U+00A5がバックスラッシュ「\」0x5Cに変換されます。
..\vulbasename.php
このため、組み立てられるパス名は以下の通りとなります。これは典型的なディレクトリトラバーサル攻撃の文字列ですね。
./data/..\vulbasename.php

文字エンコーディング変換するタイミングに注意

basename関数を通しているにもかかわらず攻撃が成立してしまう理由は、以下の通りです。
  • 円記号U+00A5は、basenameの処理対象の文字ではない
  • basenameの処理の後に、文字エンコーディング変換によりU+00A5が0x5Cに変化する
正しい手順は下記のとおりです。
  • まず文字エンコーディングを変換する
  • その後にbasename関数を通す
スクリプトとしては下記のとおりです。
$file_utf8 = $_GET['file'];
$file_sjis = mb_convert_encoding($file_utf8, 'cp932', 'UTF-8'); // 文字エンコーディング変換
setlocale(LC_CTYPE, 'Japanese_Japan.932'); // locale設定
$file = basename($file_sjis);
$path = './data/' . $file;
このスクリプトに対して先の攻撃をかけても、以下のようにスクリプトの中身は表示されません。


basenameはファイル名として妥当な文字種のチェックをしない

basename関数のソースを読むとすぐ分かりますが、basename関数がチェックするのはスラッシュ(全プラットフォーム共通)、バックスラッシュとコロン(Windows等)のみです。特にWindowsの場合ファイル名に使える文字に制約がありますが、basenameはそのチェックはしません。長さのチェックもしません。
このため、ファイル名を外部から受け取る場合、ファイル名の仕様として以下を決めておくとよいでしょう。
  • ファイル名に用いる文字の種類
  • ファイル名を表現する文字エンコーディング
  • ファイル名の長さの最小値・最大値
特に、外部から受け取ったファイル名でファイルを新規作成する場合は、受け取ったファイル名が仕様を満たすことをバリデーションで確認する必要があります。既存のファイルをオープンするだけであれば、バリデーションが必須かどうかは悩ましいところですが、一般論として入力値が前提条件(仕様)を満たすことのチェックとしてバリデーションはしておくべきだと思います。

まとめ

PHPのbasename関数を用いる上での注意点を説明しました。まとめると以下のようになります。
  • 文字エンコーディングの変換はbasenameを通す前に行うこと
  • basename関数を呼ぶ前にlocaleを設定すること
  • ファイル名の仕様を決める
  • ファイル名が仕様を満たすかどうかバリデーションにより確認する

2015年2月7日土曜日

GHOST脆弱性を用いてPHPをクラッシュできることを確認した

GHOST脆弱性について、コード実行の影響を受けるソフトウェアとしてEximが知られていますが、PHPにもgethostbynameという関数があり、libcのgethostbyname関数をパラメータ未チェックのまま呼んでいます。そこで、PHPのgethostbynameを用いることでPHPをクラッシュできる場合があるのではないかと考えました。

試行錯誤的に調べた結果、以下のスクリプトでPHPをクラッシュできることを確認しています。CentOS6(32bit/64bitとも)、Ubuntu12.04LTS(32bit/64bitとも)のパッケージとして導入したPHPにて確認しましたが、phpallで確認した限りPHP 4.0.2以降のすべてのバージョンのPHPで再現するようです。なぜかPHP 4.0.0と4.0.1では再現しませんでした。
<?php
    gethostbyname(str_repeat('0', 1027));
    gethostbyname(str_repeat('0', 1028));
CentOS6.5(32ビット)での実行の様子を下図に示します。
$ php gethostbyename-vul.php
*** glibc detected *** php: realloc(): invalid next size: 0x08b0b118 ***
======= Backtrace: =========
/lib/libc.so.6[0x92de31]
/lib/libc.so.6[0x9330d1]
/lib/libc.so.6(realloc+0xdc)[0x93326c]
/lib/libc.so.6(__nss_hostname_digits_dots+0x373)[0x9b5d13]
/lib/libc.so.6(gethostbyname+0x9a)[0x9bab6a]
php[0x8153b01]
php[0x8260729]
php(execute+0x1ce)[0x8236f4e]
php(zend_execute_scripts+0x66)[0x820f0c6]
php(php_execute_script+0x1e6)[0x81b5b76]
php[0x829feeb]
/lib/libc.so.6(__libc_start_main+0xe6)[0x8d3d26]
php[0x80622d1]
======= Memory map: ========
00101000-00129000 r-xp 00000000 fd:00 1053082    /usr/lib/libsmime3.so
00129000-0012b000 r--p 00027000 fd:00 1053082    /usr/lib/libsmime3.so
【中略】
00bee000-00bef000 rw-p 00007000 fd:00 1062164    /usr/lib/php/modules/json.so
アボートしました (コアダンプ)
$
上記はコンソール上での実行ですが、Webからの呼び出しでもPHPがクラッシュします。
このサンプルではgethostbynameを2回呼んでいますが、メモリの割り当て状況によっては1回の呼び出しでクラッシュさせることもできるでしょう。上記はあくまでPoCです。

既存のアプリケーションに対して外部からの攻撃によりPHPをクラッシュさせる、さらには任意のコードを実行させるのは困難と予想しますが、影響が皆無というわけではないでしょうから、以下を確認しておくと安全です。
  • GHOST脆弱性に対するパッチを適用していれば問題ない
  • PHPのgethostbyname関数を呼び出していなければ問題ない
  • gethostbyname関数を呼び出していても、引数を外部から制御できなければまず問題ない
  • gethostbyname関数を呼び出し、かつ引数が外部から制御できるが、バリデーションにより1024バイト以上の引数を禁止していれば問題ない
  • PHP 5.6.6、PHP 5.5.22(現在はRC1)からはgethostbynameの引数が255バイトまでに制限されるので問題ない
すなわち、gethostbynameの引数を外部から制御できるアプリケーションがある場合は、GHOST脆弱性に対するパッチを適用する(強く推奨)か、どうしてもすぐにパッチを出来ない場合は、gethostbynameの引数をバリデーションにより255バイト以下等に制限することを推奨します。あるいは、PHP5.6.6、PHP5.5.22が公開された後にこれらにバージョンアップすることでも解決します。

2015年2月3日火曜日

EximのGHOST脆弱性の影響とバリデーションの関係

追記(2015/2/6)

大垣さんから訂正依頼のコメントを頂いておりますので合わせてお読みください。徳丸としては特に訂正の必要は感じませんでしたので、本文はそのままにしています。そう思う理由はコメントとして追記いたしました。 (追記終わり)

大垣さんのブログエントリ「GHOSTを使って攻撃できるケース」を読んだところ、以下のようなことが書いてありました。
1. ユーザー入力のIPアドレス(ネットワーク層のIPアドレスではない)に攻撃用データを送る。
2. バリデーション無しで攻撃用の不正なIPアドレスをgethostbyname()に渡される。
3. ヒープオーバーフローでヒープ領域のメモリ管理用の空きサイズを改竄する。

【中略】

どんなソフトウェアが危ないのか?

  • ユーザー入力のIPアドレスをバリデーションしないでgethostbyname()を使用している。
  • インタラクティブな動作を行っている。(攻撃者からの入力に対してレスポンスがある)
  • ソフトウェアが持つ実行機能が利用できる。
【中略】

どうすれば守れたのか?

EximはメールサーバーなのでIPアドレスはネットワーク層からだけでなく、ユーザー入力としても処理しています。IPアドレスとしてあり得ないデータをアプリでエラーにしていれば攻撃はできませんでした。入力データとしてあり得ないデータをライブラリに直接渡すとリスクが伴います。入力バリデーションを行っていれば問題となりませんでした。
  • 入力バリデーションを行い、おかしな入力は拒否する
glibcの脆弱性の有無に関わらず、これだけしていれば任意コマンドの実行という最悪の事態は防げました
この記事には以下の問題があると感じました。
  • 攻撃経路となる入力値はIPアドレスではなくホスト名である
  • 攻撃を防げるとするバリデーションの仕様が明記されていない
  • Eximは本当にバリデーションをしていないのか
以下、説明します。

攻撃経路となる入力値はIPアドレスではなくホスト名である

Qualysの発表したEximに対する攻撃手法によると、攻撃はSMTPのHELOコマンドを経由して、数字とドットのみの「ホスト名」をgethostbyname()に送ることが行われます。GHOST脆弱性の性格から、攻撃に用いる文字は数字とドットのみですが、HELOコマンドが受け取るのはホスト名ですから、IPアドレスとしてバリデーションするわけにはいきません。また、gethostbyname()が受け取る引数もbynameとあるようにホスト名です。
大垣さんは、攻撃文字列が数字のみなのでこれをIPv4形式のIPアドレスと誤認されたようですが、これは間違いということになります。

攻撃を防げるとするバリデーションの仕様が明記されていない

前項と関連しますが、大垣さんは、攻撃が防げるとするバリデーションの要件を明示していません。おそらく、IPv4としてのバリデーションを想定しておられたのでしょうが、これは間違いですので、前提条件が崩れたことになります。
それでは、「大垣さんだったらホスト名に対してどんなバリデーションをするだろうか」と考えてみましたが、私が勝手に考えても失礼にあたると思いますので、大垣さんの著書「Webアプリセキュリティ対策入門 ~あなたのサイトは大丈夫?」から、似て非なる例としてメールアドレスのバリデーション関数を下記に引用します。
function validate_email($str, $check_dns=true, $mode=V_EXIT) {
  // preg_matchもバイナリセーフ
  $error = !preg_match ('/^(([a-zA-Z0-9])+([a-zA-Z0-9\._-])*@([a-zA-Z0-9_-])+([a-zA-Z0-9\._-]+)+)$/', $str);
  list(,$domain) = split('@',$str);
  // checkdnsrr()はWindowsでは実装されていません。
  if (!$error && !strstr(PHP_OS,'WIN') && !checkdnsrr($domain,'MX')) {
    $error=false;
  }
  if ($mode == V_RETURN) {
    return $error;
  }
  if ($error) {
    trigger_error('不正なメール形式を検出しました。');
  }
}
preg_matchの中の赤字の部分がドメイン名のチェックです。正規表現がRFC準拠でないことや、ドメインパートにアンダースコアを許容していることや、量化子「+」がネストしていることが気になる人もいるでしょうが、本題ではないので流します。
この関数は、メールアドレスを正規表現でチェックした後、PHPのcheckdnsrr関数を用いて、ドメイン名に対するMXレコードが存在することをチェックしています。これは、大垣さんの主張であるgethostbyname()を呼ぶ前にバリデーションを行うようにという処理とよく似ていますし、ホスト名やドメイン名のチェックを外部APIを呼び出して行う点もよく似ていますので、ここでは上記の正規表現が、「大垣流のドメイン名(ホスト名)バリデーション」と想定して議論を行います。
Qualysのアドバイザリによると、GHOST攻撃に用いる文字列は数字とドットだけからなる長い(1Kバイトを超える(後述))文字列ですが、上記の正規表現はこの攻撃文字列を許容します。すなわち、上記のバリデーションではGHOST攻撃を防げません
したがって、「入力値をバリデーションすれば攻撃を防げる」というものではなく、当然のことながら、バリデーションの仕方によって防御の効果は変わってきます

Eximは本当にバリデーションをしていないのか

大垣さんは、「バリデーション無しで攻撃用の不正なIPアドレスをgethostbyname()に渡される」と、Eximはgethostbyname()に渡す文字列をバリデーションしていないと断定していますが、本当にそうでしょうか。
Eximのソースコードおよびデバッグログを用いて確認したところ、Eximはgethostbyname()を呼ぶ前に、smtp_in.c内のcheck_helo()関数でホスト名をチェックしています。
static BOOL
check_helo(uschar *s)
{
  // 中略
  if (*s == '[')
    /* 中略 IPv6形式のIPアドレスの処理 */
  else if (*s != 0)
    {
    yield = TRUE;
    while (*s != 0)
      {
      if (!isalnum(*s) && *s != '.' && *s != '-' &&
          Ustrchr(helo_allow_chars, *s) == NULL)
        {
        yield = FALSE;
        break;
        }
      s++;
      }
    }
  }
// 後略
赤字で示したように、HELOコマンドのパラメータ(ホスト名)として、英数字、ドット、ハイフン、helo_allow_chars配列の文字(デフォルトは空だが設定により変更可能)のみを許可しています。
上記は、細かい違いはあるものの、大垣さんのvalidate_email()関数のドメインパートのバリデーションロジックと概ね同じです。
このように、Eximはホスト名をバリデーションすることなくgethostbyname()に渡しているとする大垣さんの記述は間違いです。おそらく、前述のように、gethostbyname()に渡すパラメータをホスト名ではなくIPアドレスと勘違いされていたので、「IPアドレスとしてのバリデーションをしていない」と推測されたものと思いますが、ソフトウェアのソースコードや動作を確認することなく「Eximはバリデーションしていない」と断定したことは軽率な行為であると考えます。

どうすればよかったか

Qualysのアドバイザリでは、攻撃に用いる文字列(gethostbynameの引数)の要件として下記の記述があります。
It must be long enough to overflow the buffer. For example, the non-reentrant gethostbyname*() functions initially allocate their buffer with a call to malloc(1024) (the "1-KB" requirement).
Eximが呼び出しているのは、まさに the non-reentrant gethostbyname*() functions ですので、攻撃には1KB以上の長い文字列が必要ということになります。

一方、ホスト名の長さについては、RFC1123には以下の記述があります。
Host software MUST handle host names of up to 63 characters and SHOULD handle host names of up to 255 characters.
試訳
ホストソフトウェアは63文字までのホスト名を扱わなければならず(MUST)、255文字までのホスト名を扱うべき(SHOULD)である。
すなわち、ホスト名の上限は255文字としていればRFC上問題はないため、積極的に255文字を超えるホスト名をエラーにしていれば、GHOST脆弱性の影響は受けないことになります。

とは言うものの、RFC1123の要件としては、256文字以上のホスト名を扱ってもとくに問題はないように思えます。すなわち、ホスト名の上限の仕様には以下の選択肢があることになります。
  • 63文字(MUST)
  • 255文字(SHOULD)
  • 256文字以上の有限値(任意)
  • 無制限(任意、Eximが該当)
どれを選択するかはアプリケーション要件です。長さを短くすると利便性は減ります(短所)が、未知の攻撃の際に *たまたま* 防御できる可能性がでてくる(長所)ということです。そして、Eximはホスト名の長さ制限を設けていなかったためホスト名の自由度は高い(長所)ものの、他の諸々の要因とあいまってGHOST脆弱性の受けてしまった(短所)ということです。

現実問題として、255文字を超えるホスト名を扱うことはまずないでしょうから、長さの制限を設けておけばよかったのになぁとは思います。

まとめ

EximがGHOST脆弱性の影響を受ける問題とバリデーションとの関係について説明しました。Eximは、まったくバリデーションをしていなかったわけではなく、ホスト名の文字種制限(バリデーション)はしていましたが、長さの制限はしていません。そして、長さを適当に(255文字以下、あるいは1023文字以下)制限していれば、GHOST脆弱性の影響は受けませんでした。

では、バリデーションはどのように考えればよいでしょうか。従来から言っていることですが以下を推奨します。
  • バリデーションの基準はアプリケーションの仕様である
  • 仕様を満たさない入力値は再入力を促すナビゲーションを行う
  • アプリケーション仕様として、すべての入力パラメータの以下項目を定義しておく
    • 文字種
    • 文字列長の最小・最大値
    • 数値の場合は最小値・最大値
    • メールアドレス等は書式
文字列長に関しては、制限をとくに設けていないアプリケーションも多いと思いますが、実用性を損なわない範囲で上限値を定めて、その上限まで正常に動作することと、上限値を超えた場合にエラーになることをテスト項目に加えるとよいでしょう。
バリデーションのセキュリティ上の効果については以下のように考えます。
  • バリデーションの基準はアプリケーション仕様(再掲)
  • Eximの場合のように、バリデーションの仕方によっては脆弱性が顕在化しなかったという例は結構ある(参考: Drupalの例
  • しかし、バリデーションは確実な対策ではないので、脆弱性対策をおろそかにしてはいけない
  • そのため開発者の心構えとしては、バリデーションによる防御効果はないものとして脆弱性対策をすること
  • バリデーションはしておこう。それがセーフティネットになる場合もある

フォロワー