Not only is the Internet dead, it's starting to smell really bad.:2018年05月03日分

2018/05/03(Thu)

[プログラミング] ひたすら MiniDLNA(ReadyMedia) のコードにダメ出ししていく(その1)

早々にバグ(というかクソ仕様)が特定できてしまい不完全燃焼、それならガソリン撒いて跡形も無く燃やしてしまえばいいと、所沢山賊隊こと西武打線がワイに囁いている。 BGMは Big Black/Keroseneでお願いします。

あ、NETGEAR R6300v2同梱版はもう窓から投げ捨てたので今回からはgit repositoryの最新版ベースで。

@ディレクトリの走査には必要が無い限り scandir(3) でなく opendir(3) + readdir(3)を使う

まず指定ディレクトリ以下の子要素を走査するに scandir(3) を使ってるのって微妙だねー感がある。

721 static void
722 ScanDirectory(const char *dir, const char *parent, media_types dir_types)
723 {
...
735                         n = scandir(dir, &namelist, filter_avp, alphasort);

そもそも scandir(3) って

  • 第1引数で指定されたディレクトリ以下に存在するファイル一覧を
  • 第3引数で指定された条件にマッチするファイルのみ選んで
  • 第4引数で指定されたソート関数を使って並び替え
  • malloc(3)によってメモリ確保した配列に↑の結果を格納し、第2引数にポインタをセットして返却する

という関数なので

  • 同一ディレクトリの下に
  • 大量にファイルが置かれる

みたいなユースケースには不向き、そして音楽ファイルや画像ファイル置き場なんてのは正にそういう使い方なのよね。

なので"Mini"を自称するのであれば opendir(3) + readdir(3) を使うべきとこかな。

サンプルコードだけど、↓は

#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
int
main(void)
{
	struct dirent **ent;
	int len, i;

	len = scandir(".", &ent, NULL, NULL);
	if (len < 0)
		abort();
	for (i = 0; i < len; ++i) {
		if (ent[i]->d_name[0] == '.')
			continue;
		printf("%s\n", ent[i]->d_name);
	}
	free(ent);
	exit(EXIT_SUCCESS);
}

↓と書き直せる。

#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
int
main(void)
{
	DIR *d;
	struct dirent *ent;

	d = opendir(".");
	if (d == NULL)
		abort();
	while ((ent = readdir(d)) != NULL) {
		if (ent->d_name[0] == '.')
			continue;
		printf("%s\n", ent->d_name);
	}
	exit(EXIT_SUCCESS);
}

そもそも scandir(3) の中身って opendir(3) + readdir(3) だし、参照 src/lib/libc/gen/scandir.c

 85 int
 86 scandir(const char *dirname, struct dirent ***namelist,
 87     int (*selectfn)(const struct dirent *),
 88     int (*dcomp)(const void *, const void *))
 89 {
...
 97         if ((dirp = opendir(dirname)) == NULL)
 98                 return -1;
...
108         while ((d = readdir(dirp)) != NULL) {

よって第4引数で指定するソート関数を使う用事が無いなら無駄でしかない。

元コードはソート関数として alphasort(3) を指定してるので、敢えてscandir(3)使った可能性は残ってる。alphasort(3) は

  • ファイル名をstrcoll(3)で比較し
  • qsort(3)でソートする

という関数でGNU拡張、strcoll(3)はLC_COLLATEつまり文字照合順序に応じてソートを実行する。

よって迂闊に書き換えてしまうと処理の順序が変わってしまうんだけど、ここで冷静に脳味噌を働かせて考えると

  • ソートしたいのはフォルダ名でなくID3タグ中の曲名やアーティスト名なので、ここで alphasort(3) 指定しても全く意味が無い
  • もちろん「Brows Folders」の検索結果はフォルダ名でソートされてる必要があるけど、それは今ソートする必要はゼロ
  • SQLite3 という RDBMS を使うことによるメリットを存分に享受して出力時に ORDER BY 使えばいいだけの話

ということ、やっぱりクソコードやんけ!

そもそも英語圏だとアーティスト名のソートって定冠詞(The)を無視する必要があったり、日本語圏なら漢字コード順でなく読み仮名順でのソートしないとならない。 なのでその辺ちゃんと実装しようと思ったら大変なんですよな。

@もし移植性に目を瞑れるのであれば opendir(3) + readdir(3) ではなく fts(3) の使用も検討する

readdir(3) は opendir(3) したディレクトリ直下の情報しか帰ってこないので、サブディレクトリを再帰的に処理したければコードも再帰を使って書く必要があり、ヘタクソなコードだとスタックオーバーフローの温床になりがち。

もっと高級言語だと指定したディレクトリ以下を再帰的に走査し要素をイテレーターとして返すAPIがありユーザーが再帰コード書く必要が無い、例としては

あたりがそう、実はCでも標準ではないんだけど似たような関数があってそれが表題の fts(3)なんよね。

簡単なコード例を示すと

#include <sys/types.h>
#include <sys/stat.h>
#include <fts.h>
int
main(void)
{
	FTS *d;
	FTSENT *ent;
	char *paths[] = { ".", NULL };

	d = fts_open(paths, FTS_PHYSICAL, NULL);
	if (d == NULL)
		abort();
	while ((ent = fts_read(d))) {
		if (ent->fts_level == FTS_ROOTLEVEL)
			continue;
		switch (ent->fts_info) {
		case FTS_D:
		case FTS_F:
			printf("%s\n",  ent->fts_name);
			break;
		}
	}
	exit(EXIT_SUCCESS);
}

みたいに使う、メリットとしては

  • バグの温床になりがちな再帰コードを書かずとよい
  • FTSENT には struct dirent だけでは取れないような情報も定義されてるので 改めて lstat(2) 呼ばんでも済む(TOCTTO対策だけは忘れずに)
  • fts_open(3) の第3引数には scandir(3) みたいにソート関数も指定できる

という至れり尽くせりの高機能、その分だけ仕様が複雑ではあるんだけど使うだけの価値はある *1

それに移植性は無いといえども

で動きゃ充分よね、AIXとHP-UXは隅っこで腹筋してろWindowsは好きな寿司ネタでも書いとけ。

@fts(3) ほど高機能である必要が無いのであれば、移植性のある nftw(3) を使う

つーか本当は POSIX に fts(3) によく似た nftw(3) - New File Tree Walkという関数があって、広い移植性を考えたらそっちを使う方が好ましい。

#include <sys/stat.h>
#include <sys/types.h>
#include <ftw.h>
#include <stdio.h>
#include <unistd.h>

static int
do_walk(const char *path, const struct stat *st,
    int flag, struct FTW *ftwp)
{
	switch (flag) {
	case FTW_D:
	case FTW_F:
		printf("%s\n", path);
		break;
	}
	return 0;
}

int
main(void)
{
	nftw(".", &do_walk, sysconf(_SC_OPEN_MAX), 0);
	exit(EXIT_SUCCESS);
}

ただし

  • コールバック関数(コード例だとdo_walk)にユーザー定義のパラメータ渡す方法が無い
  • グローバル変数あるいはスレッドローカル変数を使わざるをえない

というデザインが単純にダメ *2、そもそも中身 fts(3) の wrapper として実装されてるから使う意味が…

元々 fts(3) は POSIX 入る予定だったんだけどねぇ、マニュアルにも恨み言が書いてあったり。

 STANDARDS
      The fts utility was expected to be included in the IEEE Std 1003.1-1988
      (``POSIX.1'') revision.  But twenty years later, it still was not
      included in the IEEE Std 1003.1-2008 (``POSIX.1'') revision.

というか流し読みしてるとPOSIX:2008に入ってるように空目するからSTANDARDSに書くべくじゃねぇなこれ、CAVEANTSなりBUGSに移せや。

@特に理由が無いならシンボリックリンクは安全の為に無視する

次に気になったのが以下のコード

629 static int
630 filter_type(scan_filter *d)
631 {
632 #if HAVE_STRUCT_DIRENT_D_TYPE
633         return ( (d->d_type == DT_DIR) ||
634                  (d->d_type == DT_LNK) ||
635                  (d->d_type == DT_UNKNOWN)
636                 );
637 #else
638         return 1;
639 #endif
640 }

scandir(3) のフィルタに指定されてる関数のひとつなんだけど、DT_LNK つまりシンボリックリンクを除外してないのよね。

シンボリックリンクを無視するしないはあくまでアプリケーション側のポリシーの問題だけど、いったん受け入れると決めたなら注意深く実装しないと

  • シンボリックリンクが循環参照していると再帰しながらディレクトリを走査する関数なんかでスタックオーバーフローが発生する可能性がある
  • 本来はお見せしてはならないファイルにシンボリックリンクを貼るなどの方法で重要情報が流出する可能性がある

という可用性や機密性に関するセキュリティ問題が容易く発生するんですわ。

正直なところ音楽ファイルにシンボリックリンクが意味を持つとは思えないよね、ファイルシステム層でなくアプリケーション層つまりプレイリスト機能でいくらでもエイリアス切れるわけでな。 なのでこれはどんな判断だと思うけど、記事のネタ的にはシンボリックリンク扱う方が含蓄あるので以下その線で話を進める。

@どうやってシンボリックリンクの循環参照を検出するか

MiniDLNAでは↓のコードで実現してるらしい

482 int
483 resolve_unknown_type(const char * path, media_types dir_type)
484 {
...
490     if( lstat(path, &entry) == 0 )
491     {
492         if( S_ISLNK(entry.st_mode) )
493         {
494             if( (len = readlink(path, str_buf, PATH_MAX-1)) > 0 )
495             {
496                 str_buf[len] = '\0';
497                 //DEBUG DPRINTF(E_DEBUG, L_GENERAL, "Checking for recursive symbolic link: %s (%s)\n", path, str_buf);
498                 if( strncmp(path, str_buf, strlen(str_buf)) == 0 )
499                 {
500                     DPRINTF(E_DEBUG, L_GENERAL, "Ignoring recursive symbolic link: %s (%s)\n", path, str_buf);
501                     return type;

うーんこの、全然できとらんやんけ!

$ mkdir foo
$ cd foo
$ ln -sf . foo
$ ls -all foo
lrwxr-xr-x  1 tnozaki  tnozaki  1 May  2 12:00 foor -> .

みたいな循環参照は検出できない、検出可能なのは

$ ln -sf foo foo
$ ls -all foo
lrwxr-xr-x  1 tnozaki  tnozaki  1 May  2 13:14 foo -> foo

みたいにシンボリックリンクが自分自身を参照してるケースだけだわ、これ実体がはじめから存在しないから循環参照ではなく参照切れのケースですわ。

実はさっきの fts(3)を使うメリットにはもう一つあって、循環参照あるいは参照切れを回避しつつリンク先を処理するのも簡単なのよね。オプションに

     FTS_LOGICAL     This option causes the fts routines to return FTSENT
                     structures for the targets of symbolic links instead of
                     the symbolic links themselves.  If this option is set,
                     the only symbolic links for which FTSENT structures are
                     returned to the application are those referencing non-
                     existent files.  Either FTS_LOGICAL or FTS_PHYSICAL must
                     be provided to the fts_open() function.

をセットすれば簡単にチェックできる、fts_read(3) で返ってきた FTSENT の fts_info フィールドに

  • FTS_SLNONE … シンボリックリンクの参照先が存在しない
  • FTS_DC … ディレクトリが循環参照している

がついてたらビンゴなので、エラーなり警告なり無視なりすればいいだけ。

#include <sys/stat.h>
#include <sys/types.h>
#include <fts.h>
#include <stdio.h>

int
main(void)
{
	FTS *d;
	FTSENT *ent;
	char *paths[] = { ".", NULL };

	d = fts_open(paths, FTS_LOGICAL, NULL);
	if (d == NULL)
		abort();
	while ((ent = fts_read(d))) {
		if (ent->fts_level == FTS_ROOTLEVEL)
			continue;
		switch (ent->fts_info) {
		case FTS_SLNONE:
			fprintf(stderr, "ignore symlink without target: (%s)\n", ent->fts_name);
			break;
		case FTS_DC:
			fprintf(stderr, "ignore recursive symlink: (%s)\n", ent->fts_name);
			break;
		}
	}
	fts_close(d);
	exit(EXIT_SUCCESS);
}

これ ntfw(3) の方だと

  • リンク切れは FTW_SLN が帰ってくるので fts(3) と同様の処理ができる
  • 循環参照は安全の為に「完全に静かに無視」されるのでコールバック自体が呼ばれない
  • そのため↑の検出目的には使えない

ちゅーかんじ。

@次回

シンボリックリンク攻撃に対する注意を書こうかと思ってるけど、いつもの通り予定は未定。

*1:詳しい使い方は ls(1)find(1)の実装なんかで使われてるのでそっち読んでくだしあ。
*2:これでもftw(3)を廃止してのnftw(3)なんだよなぁ…