Barbarism begins at internet:2021年11月02日分

2021/11/02(Tue)

[オレオレN6] sed(1) -iが作成する一時ファイルってTOCTTOU対策してなくない?

以前オレオレN6の sed(1) size_t underflowバグを退治した時、ついでに他の*BSDなんかに実装されてるGNU由来の-iすなわちinplace editingオプションをバックポートしたのだけど、-iに引数を渡さないすなわちバックアップファイルを残さない指定が動いてないのに気づいてしまった。

$ cat aaa >unko.txt
$ sed -i -e "s/aaa/bbb/" unko.txt
$ ls
$ unko.txt
$ unko.txt-e

ご覧の通りように-eが-iの引数と解釈されてしまっておるのだ。

これはなぜかというと、そもそも*BSDの伝統的なgetopt(3)ってのはoptstringに

しか指定できないので、そもそも実装は無理ゲーでGNU拡張である

を使わないとならないのよな。

んで今では*BSDのgetopt(3)実装もこのGNU拡張を扱えるような変更が入ってるので、オレオレN6もバックポートすりゃいいのでたいした話ではない。 ほんの数行持ってくるだけだし、なんならバックポートせんでもgetopt_long(3)を使ったってよい。

なおNのgetopt_long(3)にはバグあって ショートオプションを使うと死ぬので別の問題が発生するのだがそういやすっかり放置してた(互換性問題があるので)、そのうちOの実装に置き換えますね。

そんでこっからが本題、この-iオプション実装(最初に実装したのはFの誰か)の中身をまじめに読んだところ

406	                if (inplace != NULL) {
407	                        if (lstat(fname, &sb) != 0)
408	                                err(1, "%s", fname);
409	                        if (!S_ISREG(sb.st_mode))
410	                                errx(1, "%s: %s %s", fname,
411	                                    "in-place editing only",
412	                                    "works for regular files");
413	                        if (*inplace != '\0') {
414	                                strlcpy(oldfname, fname,
415	                                    sizeof(oldfname));
416	                                len = strlcat(oldfname, inplace,
417	                                    sizeof(oldfname));
418	                                if (len > (ssize_t)sizeof(oldfname))
419	                                        errx(1, "%s: name too long", fname);
420	                        }
421	                        if ((dirbuf = strdup(fname)) == NULL ||
422	                            (basebuf = strdup(fname)) == NULL)
423	                                err(1, "strdup");
424	                        len = snprintf(tmpfname, sizeof(tmpfname),
425	                            "%s/.!%ld!%s", dirname(dirbuf), (long)getpid(),
426	                            basename(basebuf));
427	                        free(dirbuf);
428	                        free(basebuf);
429	                        if (len >= (ssize_t)sizeof(tmpfname))
430	                                errx(1, "%s: name too long", fname);
431	                        unlink(tmpfname);
432	                        if (outfile != NULL && outfile != stdout)
433	                                fclose(outfile);
434	                        if ((outfile = fopen(tmpfname, "w")) == NULL)
435	                                err(1, "%s", fname);
436	                        fchown(fileno(outfile), sb.st_uid, sb.st_gid);
437	                        fchmod(fileno(outfile), sb.st_mode & ALLPERMS);
438	                        outfname = tmpfname;
439	                        if (!ispan) {
440	                                linenum = 0;
441	                                resetstate();
442	                        }
443	                } else {
444	                        outfile = stdout;
445	                        outfname = "stdout";
446	                }

はい注目、424~426行目で一時ファイルのパス名の組立てを行ってるのだけど

<元ファイルのディレクトリ>/.!<プロセス番号>!<元ファイル名>

として作ってるのだよね、これランダム部分がプロセスIDしかねぇからよゆーで予測可能だよな。

このコードを読んでなにがわるいの?な人はTOCTTOUでググれ、当チラシの裏でも過去にまず危険性について ここ、そして一時ファイルとして作られるファイルが予測可能であることの危険性について ここでざっくり解説してるので割愛する。

これなぁ431行目で先にunlink(2)することでsymlink attack封じてるつもりなんだろうか、アトミック操作じゃねえのだからfopen(3)までのわずかな時間にsymlink貼るのに成功したらダメだよね。

なおOはFに日常的に放り込まれるクソコードを精査せずにそのまま持ってくるような愚はしないので、ちゃんとmkstemp(3)使うよう書き換えとる、正しい。

396			strlcpy(dirbuf, fname, sizeof(dirbuf));
397			len = snprintf(tmpfname, sizeof(tmpfname),
398			    "%s/sedXXXXXXXXXX", dirname(dirbuf));
399			if (len >= sizeof(tmpfname))
400				error(FATAL, "%s: name too long", fname);
401			if ((fd = mkstemp(tmpfname)) == -1)
402				error(FATAL, "%s: %s", fname, strerror(errno));
403			if ((outfile = fdopen(fd, "w")) == NULL) {
404				unlink(tmpfname);
405				error(FATAL, "%s", fname);
406			}

そんでGNU sedの方もだけど、あたりまえにmkstemp(3)使ってるよね。

571   if (in_place_extension)
572     {
…
617       output_file.fp = ck_mkstemp (&input->out_file_name, tmpdir, "sed",
618                                    write_mode);

めんどくさいのでラッパーの先までは読む気ナッシング。

(追記) Nの場合sed(1)はクロスコンパイル環境用のツール(HOST TOOLS)にも入ってるので、mkstemp(3)の無いホスト環境だとbuild.sh toolsに失敗するので、-iオプション自体を-DHOSTPROGの場合は含めないように書き直した方がいいかも。でもMakefile中で${TOOL_SED} -i使ってる箇所結構ありそうだな。

というかHOST TOOLSは全部C90でコンパイルできるようにしろとかREADMEに無茶を平気で書くくせにHOST TOOLSを使うときのオプションはPOSIX縛りにしないとか雑なのである。