Namazu-devel-ja(旧)

[Tadamasa Teranishi <yw3t-trns@xxxxxxxxxxxxxxx>]

寺西です。

Tadamasa Teranishi wrote:
>  
> namazu-cgi-9 がセグメンテーションフォールトを起こす問題が以前あった
> かと思いますが、当方でも再現しました。
...
> src/rcfile.c
> 
>     char tmp[BUFSIZE];
> 
>     char *home;
>     if ((home = getenv("HOME")) != NULL) {
>       strncpy(tmp, home, BUFSIZE - 1);
>       strncat(tmp, "/", BUFSIZE - strlen(tmp) - 1);
...
> getenv("HOME") で返される文字列の長さが BUFSIZE - 1 を超えた場合、
> まず、strncpy() で BUFSIZE - 1 分コピーされます。
> しかし、'\0' のターミネート文字がありません。
> このため、次の strncat() の strlen(tmp) の値は BUFSIZE - 1 を超える
> 場合があります。すると、BUFSIZE - strlen(tmp) - 1 が負の数に
> なります。tmp の文字列は '\0' でターミネートされていない上、
> 負の数字で strncat() を実行するので、strncat() を実行した時点で
> メモリを破壊します。

どうやら char tmp[BUFSIZE]; の宣言で配列の中身が 0 クリアされる
ことを期待したコードのようです。
# グローバル変数、static 変数は明示的な初期化をしていない場合は
# 0 クリアされますが、ローカル変数はクリアされません。

0 クリアされているなら、tmp[BUFSIZE - 1] は '\0' になるため、
strlen(tmp) は、BUFSIZE - 1 までの値を必ず取ることになり、
BUFSIZE - strlen(tmp) - 1 は、0 以上の値になります。

strncpy(), strncat() の第三引数が 0 で大丈夫かどうかは、ちょっと
自身がないのですが手元の環境では 0 でも大丈夫でした。

また、tmp[BUFSIZE - 1] は書き変わらないため '\0' が入っており、
strlen(tmp) は BUFSIZE - 1 までの値になります。

今回は、大幅な修正を行わなくてもよいために
　　
     char tmp[BUFSIZE] = "";

と初期化することにしました。
元々、同等の処理を行っている箇所もありましたので、それに合わせ
ました。

手元の環境では BUFSIZE より短い文字列で初期化した場合、初期化文字列
以下は 0 クリアされました。(これは C の仕様だったかどうか忘れました。
たぶん大丈夫だと思いますが...。)

また、fgets() では第二引数に 0 を与えるとメモリを壊しましたので、
こちらについても修正しました。

> これはかなりまずい問題ではないかと思います。
> 
> 同様の処理は沢山ありそうです。

とりあえず簡単なバグは取ったつもりですが、完全ではないと思います。

commit しましたので、チェックできる方はチェックしていただけると
助かります。
-- 
=====================================================================
寺西 忠勝(TADAMASA TERANISHI)  yw3t-trns@xxxxxxxxxxxxxxx
http://www.asahi-net.or.jp/~yw3t-trns/index.htm
Key fingerprint =  474E 4D93 8E97 11F6 662D  8A42 17F5 52F4 10E7 D14E