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に
- "i" … 引数なし
- "i:" … 引数あり
しか指定できないので、そもそも実装は無理ゲーでGNU拡張である
- "i::" … 引数はオプション(ありなしどっちでもいい)
を使わないとならないのよな。
んで今では*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縛りにしないとか雑なのである。