Not only is the Internet dead, it's starting to smell really bad.:2014年06月27日分

2014/06/27(Fri)

[Citrus][FreeBSD][Security] FreeBSD-SA-14:15およびCVE-2014-3951 について (その2)

昨日の続きです。

@citrus_prop パーサにおける NULL pointer dereference 問題

FreeBSD-SA-14がこれに相当します、II. Problem Description における以下の部分です。

A NULL pointer dereference in the initialization code of the HZ module

この文章だと libHZ( citrus_hz.c)側の問題っぽいですが、実際にはこれ citrus_prop パーサ( citrus_prop.c)のバグですやね。 なのでこいつを使ってる libBIG5( citrus_big5.c) にも同様の脆弱性があります。

最初にこの文言と対策patchを見たときに、明示的に NULL を渡さない限りありえねーし必要なところには契約プログラミング *1で NULL はこないと 表明しとんのにと思いました。

以下、実際の「the initialization code of the HZ module」の コードです。

694 static int
695 _citrus_HZ_encoding_module_init(_HZEncodingInfo * __restrict ei,
696 	const void * __restrict var, size_t lenvar)
697 {
698 	int errnum;
699 
700 	_DIAGASSERT(ei != NULL);
...
705 	errnum = _citrus_prop_parse_variable(
706 	    root_hints, (void *)ei, var, lenvar);

700行目の_DIAGASSERTマクロで_HZEncodingInfo 型のポインタ ei が NULL で無いと表明しています。

ちなどーでもいいですが _HZEncodingInfo 型というのは↓こんなん。

121 typedef struct {
122 	escape_list e0, e1;
123 	graphic_t *ascii, *gb2312;
124 } _HZEncodingInfo;

前回 影響範囲で触れたように libHZ は HZ と HZ8 という2種類の符号化文字手法を扱うのですが、それぞれで振る舞いが変わる部分の情報については この型に閉包されます(要はクロージャっす)。

そんで iconv_open(3) や setlocale(3) が呼ばれると、このクロージャが先ほどの _citrus_HZ_encoding_module_init() 内で初期されます。 初期化に必要なパラメータは、引数である

  • var (バイト列)
  • lenvar (バイト列長)

がそれ。このパラメータって実体はただの文字列で LC_CTYPE database や iconv database の中で定義されてる VARIABLE ちゅーものです。

mklocale(1)のマニュアルに

     VARIABLE   This keyword must be followed by a single tab or space charac-
                ter, after which encoding specific data is placed.  Currently
                only the EUC encoding requires variable data.

と説明ありますな *2

この文字列なんですが、これまで各モジュールがてんでバラバラにパーサを実装していました。 ですがサポートする符号化文字集合が増えるにつれ、必要なパラメータも増えて手間がかかるようになったので、 ワイが libHZ を実装したときに citrus_prop という共通パーサを実装を追加したのですよな。 モジュール側で実装しなくても、hint と callback function だけマクロ使って定義しとけば良くなるので、ほんのちょっと開発が楽になるような気がします *3

こいつを利用してるのが libHZ とlibBIG5 の2つのモジュールなので、今回の NULL pointer dereference 問題はこの2つでのみ発生してる事と完全に一致。 作者であるワイにはすぐに citrus_prop パーサの不具合(あっ、察し)となるわけです。

ここまで整理すると

  • クロージャはわざわざ表明して NULL にならないことを保証している
  • パラメータも LC_CTYPE/iconv database にちゃんと定義されてるので NULL にならないことは確実
  • 問題が発生してるのは citrus_prop パーサ内部

うーんこの、どう考えても NULL pointer dereference が発生する要素が微粒子レベルでも存在しません、ワイ混乱。

@俺達はとんでもない思い違いをしていたようだ。

話は聞かせてもらった、地球は滅亡する(キバヤシAA略)

これ( iconv.patch)を見てみろ。

まず citrus_prop の関数及びコールバック関数のプロトタイプで引数の void ** を void * に変更している、これはノイズと考えられるので削除し、残りの差分を取り出す。

Index: libc/iconv/citrus_prop.c
===================================================================
--- libc/iconv/citrus_prop.c	(revision 267897)
+++ libc/iconv/citrus_prop.c	(working copy)
@@ -436,7 +436,7 @@
 			break;
 		_memstream_ungetc(&ms, ch);
 		errnum = _citrus_prop_parse_element(
-		    &ms, hints, (void ** __restrict)context);
+		    &ms, hints, (void ** __restrict)&context);
 		if (errnum != 0)
 			return (errnum);
 	}
Index: libiconv_modules/BIG5/citrus_big5.c
===================================================================
--- libiconv_modules/BIG5/citrus_big5.c	(revision 267897)
+++ libiconv_modules/BIG5/citrus_big5.c	(working copy)
@@ -179,7 +179,7 @@
 
 	if (start > 0xFF || end > 0xFF)
 		return (EINVAL);
-	ei = (_BIG5EncodingInfo *)ctx;
+	ei = (_BIG5EncodingInfo *)*ctx;
 	i = strcmp("row", s) ? 1 : 0;
 	i = 1 << i;
 	for (n = start; n <= end; ++n)
@@ -197,7 +197,7 @@
 
 	if (start > 0xFFFF || end > 0xFFFF)
 		return (EINVAL);
-	ei = (_BIG5EncodingInfo *)ctx;
+	ei = (_BIG5EncodingInfo *)*ctx;
 	exclude = TAILQ_LAST(&ei->excludes, _BIG5ExcludeList);
 	if (exclude != NULL && (wint_t)start <= exclude->end)
 		return (EINVAL);

するとできあがる差分……『ノ ス ト ラ ダ ム ス』。

つまりこのバグの本質は NULL pointer dereference なんかではなく

NetBSD から FreeBSD へ移植された時に作業者が & や * を誤って消してしまった後テストしてなくて発覚しなかった

だったんだよ!

(; ・`д・´) ナ、ナンダッテー !! (`・д´・ (`・д´・ ;)

そりゃーNetBSDで問題が発生するわけありませんわ、FreeBSD-SA-14についてワイは完全無罪の勝訴判決ガッツポーズ淫夢くん。

@次回予告

残りの FreeBSD-SA-15 も調査します、みとけよ~みとけよ~。

*1:NetBSD では 契約プログラミングが推奨されとりまして、関数の引数についての事前条件はすべて_DIAGASSERTマクロで違反が無いかをチェックしています
*2:the typical NetBSD style、内容が古いのでlibEUCでしか使ってないような事書いてありますが、今は使ってないのはlibUTF8くらいじゃなかろうか。
*3:今考えると hint とか定義すんのクッソめんどいし実装ウンコやね。前からもっとマシなものに書き直したかったんだけど後方互換の壁があってだな。 ちな正月くらいに新しいプロパティ機構実装してテストケース書くのめんどくさくて放置中、そのうち bitbacket の Portable Citrus iconv に入ります。 こっちは後方互換とか全部捨てていくスタイルなので。