Bug #11386: taint flag about rb_fstring()

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.0.0: DONTNEED, 2.1: REQUIRED, 2.2: REQUIRED

[usa (Usaku NAKAMURA)]

r51261以降、mswinのtest-allでfailureが出るようになった件を調査していて発見したのですが、
rb_fstring()にはtaintフラグを保存しないという問題があります。

原因は2つあって、

1. sharedなStringオブジェクトを登録する際にtaintフラグをコピーしないバグがある
2. 既に同じバイト列・エンコーディングで表現可能なStringオブジェクトが登録されている場合それを返すが、
   引数のtaintフラグと返すオブジェクトのtaintフラグの違いを評価していない

というものです。

前者は以下のパッチで直るのでどうでもいいんですが、後者はrb_fstring()の仕様がどうなのか、
という問題であると思います。
r51261より前のように、SymbolやRubyレベルでは見えないStringオブジェクトのみを扱っているのならば特に問題ではないのでr51261のような使い方を禁止するべきなのか、
rb_fstring()を変更してtaintフラグを適切に扱うようにすべきなのか、どちらでしょうか？

Index: string.c
===================================================================
--- string.c	(リビジョン 51334)
+++ string.c	(作業コピー)
@@ -245,8 +245,10 @@ fstr_update_callback(st_data_t *key, st_data_t *va
     }
     else {
 	if (STR_SHARED_P(str)) { /* str should not be shared */
-	    str = rb_enc_str_new(RSTRING_PTR(str), RSTRING_LEN(str), STR_ENC_GET(str));
-	    OBJ_FREEZE(str);
+	    VALUE newstr = rb_enc_str_new(RSTRING_PTR(str), RSTRING_LEN(str), STR_ENC_GET(str));
+	    OBJ_INFECT(newstr, str);
+	    OBJ_FREEZE(newstr);
+	    str = newstr;
 	}
 	else {
 	    str = rb_str_new_frozen(str);

[usa (Usaku NAKAMURA)]

• Backport changed from 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: REQUIRED to 2.0.0: DONTNEED, 2.1: REQUIRED, 2.2: REQUIRED

[nobu (Nobuyoshi Nakada)]

• File 0001-string.c-keep-taintedness.patch 0001-string.c-keep-taintedness.patch added
• File 0002-tests-for-fstring-taintedness.patch 0002-tests-for-fstring-taintedness.patch added

tainted/untaintedのfstringを分けるようにするパッチです。

[usa (Usaku NAKAMURA)]

なるほど、とは思ったのですが、
「次にぼくは『特異メソッドを定義したStringオブジェクトをrb_fstring()に渡すと問題がある』と言うッ！」
という感じです。(そして次はインスタンス変数)

[nobu (Nobuyoshi Nakada)]

• File 0003-string.c-bare-string-fstring.patch 0003-string.c-bare-string-fstring.patch added

ほいさ

[usa (Usaku NAKAMURA)]

むむ、早い。

で、このパッチだと、Stringのサブクラスのインスタンスだったり特異メソッドがあったりインスタンス変数を持ってたりするとfstringにならなくなってるわけですが、そもそもtaintフラグが立ってるものもfstringにならなくてもいいんじゃない？ 的なことも思ったりします。

というわけで、やっぱり、「fstringって本質的にはなんなの？」という仕様の問題に立ち返る気がするのですが、これを説明できるのは誰でしょう？
最初は笹田さんかと思ってたんですが、実は元々これを持ってきたのはちゃりさむ？

[nobu (Nobuyoshi Nakada)]

昨日笹田さんとも話した結果、「内容だけをfstringに保存する」という方針がいいのではないか、ということになりました。

https://github.com/nobu/ruby/tree/bug/fstring-extra

[usa (Usaku NAKAMURA)]

やっぱそうなりますよねー。
ぼくもそれがいいと思います。

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset r51360.

---

string.c: pool only bare strings in fstring

• string.c (fstr_update_callback): pool bare strings only.
• string.c (rb_fstring): return the original string with sharing a
  fstring if it has extra attributes, not the fstring itself.
  [ruby-dev:49188] [Bug #11386]

[naruse (Yui NARUSE)]

• Related to Bug #12923: Accessing singleton_class of fstring cause assertion failure added