Orz日記 by Akio Morita

_ [FreeBSD]libX11をアップデートするとXIM回りが死ぬ

x11/libX11 1.6.9_3,1以降に更新すると、XIMの初期化処理でlibX11にてSEGVする

セキュリティー目的で追加した境界チェックコードでエンバグしている模様

例えば、XIM経由で動く ktermやemacsが該当例

2020-08-04追記 libX11-1.6.10_1,1で修正された

ktermのSEGVの場合、XGetIMValues(im, XNQueryInputStyle, &im_styles, NULL)によるQuery要求に対して、libX11のmodules/im/ximcp/imRmAttr.c内 _XimAttributeToValue関数の XimType_XIMStyles型に対する処理が、if ((sizeof(num) + (num * sizeof(XIMStyle))) > data_len) return Falseで失敗するための模様

手元の事例では、num=6, data_len=28bytesで XimType_XIMStylesに対するQueryが渡ってきており、先頭のunit16_t 2語がヘッダー部なのでXIMStyleの引数列は計 24bytesとなり、6引数とすると1語 4bytesの構成となるのだが、X11/Xlib.hのXIMStyle型の定義はunsigined longなので、LP64環境では1語 8bytesの定義になっている

当該if節は、data_lenの検証をXIMStyle型 num語が渡っているとして検査しているが、読み出し側に使っている配列ポインタは、uint32_t相当のCARD32型なので 4bytesずつ舐めている計算になる。

処理している入力バッファは、XIMプロトコルでX Serverから渡されたunibyte streamと思われるので、検査コードのバグっぽぃ

あと、類似の検査を行っているXimType_XIMHotKeyTriggersのケースと比較すると、CARD16型のパディング1語分をカウントし忘れているのも、検査に成功した条件で未割り当てのメモリアクセスを許す点でバグの模様

XimType_XIMHotKeyTriggersの検査も、CARD32型の三つ組をnumセット読み出しているようなので、比較すべきコンテンツボディ長は、num * 3 * sizeof(CARD32)で定義すべきと思われる (一応、XIMHotKeyTrigger型は、KeySymとint 2個からなる構造体なので、KeySymが 32bitなら辻褄はあっているが…)

--- modules/im/ximcp/imRmAttr.c.orig    2020-07-31 22:46:13.000000000 +0900
+++ modules/im/ximcp/imRmAttr.c 2020-08-03 17:56:02.608498000 +0900
@@ -265,8 +265,8 @@

            if (num > (USHRT_MAX / sizeof(XIMStyle)))
                return False;
-           if ((sizeof(num) + (num * sizeof(XIMStyle))) > data_len)
-               return False;
+           if ((sizeof(data[0]) + sizeof(data[1]) + (num * sizeof(style_list[0]))) > data_len)
+               return False;
            alloc_len = sizeof(XIMStyles) + sizeof(XIMStyle) * num;
            if (alloc_len < sizeof(XIMStyles))
                return False;
@@ -379,7 +379,7 @@

            if (num > (UINT_MAX / sizeof(XIMHotKeyTrigger)))
                return False;
-           if ((sizeof(num) + (num * sizeof(XIMHotKeyTrigger))) > data_len)
+           if ((sizeof(num) + (num * 3 * sizeof(key_list[0]))) > data_len)
                return False;
            alloc_len = sizeof(XIMHotKeyTriggers)
                      + sizeof(XIMHotKeyTrigger) * num;