Project

General

Profile

Bug #11386

taint flag about rb_fstring()

Added by usa (Usaku NAKAMURA) almost 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-dev:49188]

Description

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);


Files

0001-string.c-keep-taintedness.patch (2.08 KB) 0001-string.c-keep-taintedness.patch nobu (Nobuyoshi Nakada), 07/22/2015 11:23 PM
0002-tests-for-fstring-taintedness.patch (2.61 KB) 0002-tests-for-fstring-taintedness.patch nobu (Nobuyoshi Nakada), 07/22/2015 11:23 PM
0003-string.c-bare-string-fstring.patch (2.26 KB) 0003-string.c-bare-string-fstring.patch nobu (Nobuyoshi Nakada), 07/23/2015 01:42 AM

Related issues

Related to Ruby trunk - Bug #12923: Accessing singleton_class of fstring cause assertion failureClosedActions

Associated revisions

Revision 4ab69ebb
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

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]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@51360 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 51360
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

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]

Revision 51360
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

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]

Revision 51360
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

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]

Revision 51360
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

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]

Revision 51360
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

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]

Revision b7068102
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

string.c: fstring must not be a shared string

  • string.c (fstr_update_callback): fstring must not be a shared string, or the content without RSTRING_FSTR may be freed. [ruby-dev:49188] [Bug #11386]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@51363 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 51363
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

string.c: fstring must not be a shared string

  • string.c (fstr_update_callback): fstring must not be a shared string, or the content without RSTRING_FSTR may be freed. [ruby-dev:49188] [Bug #11386]

Revision 51363
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

string.c: fstring must not be a shared string

  • string.c (fstr_update_callback): fstring must not be a shared string, or the content without RSTRING_FSTR may be freed. [ruby-dev:49188] [Bug #11386]

Revision 51363
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

string.c: fstring must not be a shared string

  • string.c (fstr_update_callback): fstring must not be a shared string, or the content without RSTRING_FSTR may be freed. [ruby-dev:49188] [Bug #11386]

Revision 51363
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

string.c: fstring must not be a shared string

  • string.c (fstr_update_callback): fstring must not be a shared string, or the content without RSTRING_FSTR may be freed. [ruby-dev:49188] [Bug #11386]

Revision 51363
Added by nobu (Nobuyoshi Nakada) almost 4 years ago

string.c: fstring must not be a shared string

  • string.c (fstr_update_callback): fstring must not be a shared string, or the content without RSTRING_FSTR may be freed. [ruby-dev:49188] [Bug #11386]

History

Updated by usa (Usaku NAKAMURA) almost 4 years ago

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

Updated by usa (Usaku NAKAMURA) almost 4 years ago

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

Updated by usa (Usaku NAKAMURA) almost 4 years ago

むむ、早い。

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

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

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

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

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

Updated by usa (Usaku NAKAMURA) almost 4 years ago

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

#8

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • 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]
#9

Updated by naruse (Yui NARUSE) over 2 years ago

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

Also available in: Atom PDF