Bug #5279: $SAFEが3以上の時にString#encodeがSecurityErrorを発生させるケースがある

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: -

[sorah (Sorah Fukumori)]

sora_hです．

twitter
で @hsbt (Hiroshi SHIBATA) さんがこのような事を言っていたので調査してみました:
http://twitter.com/#!/hsbt/status/110700488667832320

調査したところ，どうやらString#encodeは内部的にrequireしていて，
セーフレベル3から全てのオブジェクトが汚染されるので，rb_require_safeに渡るStringが汚染されるため，
rb_requireでSecurityErrorが発生します．

なので，以下の場合はSecurityErrorが発生しますが，

$SAFE = 3
"a".encode("UTF-16")

以下の場合は発生しません．

"a".encode("UTF-16")
$SAFE = 3
"a".encode("UTF-16")

これを修正するパッチを書いてみましたが(チケット末尾に貼り付け)，
果たしてrb_require_safeの第二引数に0を渡しても問題ないのか自信がありません．
これはセキュリティ周りの問題なので，合意を取ってからコミット，もしくはパッチに
含む問題を修正し合意が取れてからコミットしようと思います．

以下patch

diff --git a/ChangeLog b/ChangeLog
index a16e823..07f76a7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+Tue Sep 6 08:56:06 2011 Shota Fukumori [e-mail]
+

•   * transcode.c: Use rb_require_safe() to load transcoder.
  

•     Because if $SAFE is higher than 3, rb_require() raises SecurityError.
  

Mon Sep 5 20:59:30 2011 CHIKANAGA Tomoyuki [e-mail]

    * insns.def: change encoding pragma for emacs (shift_jis to utf-8).

diff --git a/transcode.c b/transcode.c
index 2c188b6..0651aec 100644
--- a/transcode.c
+++ b/transcode.c
@@ -375,7 +375,7 @@ load_transcoder_entry(transcoder_entry_t *entry)
return NULL;
memcpy(path, transcoder_lib_prefix, sizeof(transcoder_lib_prefix) - 1);
memcpy(path + sizeof(transcoder_lib_prefix) - 1, lib, len + 1);

•    if (!rb_require(path))
  

•    if (!rb_require_safe(rb_str_new2(path), 0))
         return NULL;
  

  }

[mrkn (Kenta Murata)]

「rb_require_safe の第2引数に0を渡して良いかどうか」が問題なのではなく、
「transcoder の中で一時的に safe level を0に戻して良いかどうか」が問題なのではないでしょうか。

[sorah (Sorah Fukumori)]

中身的にはそういう事ですね． < transcoder の中で一時的に safe level を 0

#ruby-ja で議論したところ，autoloadはautoloadを定義した場所のSAFEで読み込むので，
$SAFE=0のときにあらかじめautoloadを仕掛けておいたというような解釈(仕組みな全然違いますが)
をすれば別に問題ないのではないか，という感じでした．

懸念しているのはrequireのpathにスクリプトサイドから自分でパスを流す事ができるかできないかで，
自分の認識ではできないと思っているのですが，もし可能だとするとrb_require_safeの第二引数に0を
渡すのは危険なのでこの修正方法じゃダメだと思っています．

[naruse (Yui NARUSE)]

• ruby -v changed from ruby 1.9.4dev (2011-09-05 trunk 33195) [x86_64-darwin11.1.0] to -

2011年9月6日11:02 Shota Fukumori [e-mail]:

懸念しているのはrequireのpathにスクリプトサイドから自分でパスを流す事ができるかできないかで，
自分の認識ではできないと思っているのですが，もし可能だとするとrb_require_safeの第二引数に0を
渡すのは危険なのでこの修正方法じゃダメだと思っています．

見れば分かる通りこの path は transcoder_entry_t から取り出されているわけですが、
この entry がどこで作られているかというと、rb_declare_transcoder で通常作られます。
で、こいつはどこから呼ばれるかというと、transdb.h からビルド時に決定される
固定文字列で呼ばれます。
なので、安全だと考えています。
（もちろん C API たたいて横から割り込んだ時はこの限りではない）

--
NARUSE, Yui [e-mail]

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r33201.
Shota, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• encoding.c (load_encoding): predefined encoding names are safe.
  [ruby-dev:44469] [Bug #5279]
• transcode.c (load_transcoder_entry): ditto.

[sorah (Sorah Fukumori)]

じゃぁ，大丈夫かな．

P.S.: このバグについてのテストケースは今書いています．

2011/9/6 NARUSE, Yui [e-mail]:

なので、安全だと考えています。
（もちろん C API たたいて横から割り込んだ時はこの限りではない）

--
Shota Fukumori a.k.a. @sora_h - http://sorah.jp/

[kazuhiko (Kazuhiko Shiozaki)]

On 06/09/2011 06:10, Shota Fukumori (sora_h) wrote:

じゃぁ，大丈夫かな．

P.S.: このバグについてのテストケースは今書いています．

2011/9/6 NARUSE, Yui [e-mail]:

なので、安全だと考えています。
（もちろん C API たたいて横から割り込んだ時はこの限りではない）

r33201 で修正されましたが、これは1.9.3ブランチにバックポートされますか？

かずひこ

[sorah (Sorah Fukumori)]

sora_h $B$G$9!#(B

On Sep 6, 2011 7:12 PM, "Kazuhiko" [e-mail] wrote:

r33201 $B$G=$@5$5$l$^$7$?$,!"$3$l$O(B1.9.3$B%V%i%s%A$K%P%C%/%]!<%H$5$l$^$9$+!)(B

$B$5$l$k$Y$-$@$H;W$$$^$9!#$^$@(BBackport93$B%W%m%8%'%/%H$O$J$$$N$+!#!#!#(B
$B$$$^C/$,%P%C%/%]!<%HC4Ev$7$F$k$s$G$7$g$&!#(B

[kosaki (Motohiro KOSAKI)]

2011年9月6日19:14 Shota Fukumori (sora_h) [e-mail]:

sora_h です。

On Sep 6, 2011 7:12 PM, "Kazuhiko" [e-mail] wrote:

r33201 で修正されましたが、これは1.9.3ブランチにバックポートされますか？

されるべきだと思います。まだBackport93プロジェクトはないのか。。。
いま誰がバックポート担当してるんでしょう。

あるように見えるけど？

http://redmine.ruby-lang.org/projects/ruby-193

[sorah (Sorah Fukumori)]

あれれ，ほんとだ．失礼．

2011/9/6 KOSAKI Motohiro [e-mail]:

あるように見えるけど？

http://redmine.ruby-lang.org/projects/ruby-193

--
Shota Fukumori a.k.a. @sora_h - http://sorah.jp/

[sorah (Sorah Fukumori)]

かずひこさんが #5286 でバックポートチケットを切っているみたいです．
今バックポートできるのは誰なんでしょう．yuguiさん?
http://redmine.ruby-lang.org/issues/5286

2011/9/7 Shota Fukumori (sora_h) [e-mail]:

あれれ，ほんとだ．失礼．

--
Shota Fukumori a.k.a. @sora_h - http://sorah.jp/

[kazuhiko (Kazuhiko Shiozaki)]

かずひこです。

On 06/09/2011 12:10, Kazuhiko wrote:

On 06/09/2011 06:10, Shota Fukumori (sora_h) wrote:

じゃぁ，大丈夫かな．

P.S.: このバグについてのテストケースは今書いています．

2011/9/6 NARUSE, Yui [e-mail]:

なので、安全だと考えています。
（もちろん C API たたいて横から割り込んだ時はこの限りではない）

r33201 で修正されましたが、これは1.9.3ブランチにバックポートされますか？

Yuguiさんが1.9.3ブランチにバックポートしてくださって、先ほどめでたく
1.9.3RC1が出た今ころになって気づいて、ものすごく申し訳ないのです
が、$SAFE=3の時は動くけれど、$SAFE=4だと相変わらず失敗するようです。

以下、1.9.3RC1での挙動です。

$ ruby1.9 -v
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]

$ ruby1.9 -e '$SAFE=3;"a".encode("utf-16be")'
(no exception)

$ ruby1.9 -e '$SAFE=4;"a".encode("utf-16be")'
-e:1: warning: failed to load encoding (utf-16be); use ASCII-8BIT instead

$ ruby1.9 -e ' "a".encode("utf-16be");$SAFE=4;"a".encode("utf-16be")'
(no exception)

かずひこ

[kosaki (Motohiro KOSAKI)]

かずひこです。

On 06/09/2011 12:10, Kazuhiko wrote:

On 06/09/2011 06:10, Shota Fukumori (sora_h) wrote:

じゃぁ，大丈夫かな．

P.S.: このバグについてのテストケースは今書いています．

2011/9/6 NARUSE, Yui [e-mail]:

なので、安全だと考えています。
（もちろん C API たたいて横から割り込んだ時はこの限りではない）

r33201 で修正されましたが、これは1.9.3ブランチにバックポートされますか？

Yuguiさんが1.9.3ブランチにバックポートしてくださって、先ほどめでたく
1.9.3RC1が出た今ころになって気づいて、ものすごく申し訳ないのです
が、$SAFE=3の時は動くけれど、$SAFE=4だと相変わらず失敗するようです。

以下、1.9.3RC1での挙動です。

$ ruby1.9 -v
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]

$ ruby1.9 -e '$SAFE=3;"a".encode("utf-16be")'
(no exception)

$ ruby1.9 -e '$SAFE=4;"a".encode("utf-16be")'
-e:1: warning: failed to load encoding (utf-16be); use ASCII-8BIT instead

$ ruby1.9 -e ' "a".encode("utf-16be");$SAFE=4;"a".encode("utf-16be")'
(no exception)

r33328 で直ったようですが、1.9.3に入れますか？みなさんの意見が聞きたいです。
正直ぼくのなかでは重要度は微妙なんですけど、このままだた1.9.[234] でそれぞれ
仕様が違うという事態になるので、それもいかがなものかと思い悩んでいます。

[shugo (Shugo Maeda)]

前田です。

2011年9月26日22:26 KOSAKI Motohiro [e-mail]:

Yuguiさんが1.9.3ブランチにバックポートしてくださって、先ほどめでたく
1.9.3RC1が出た今ころになって気づいて、ものすごく申し訳ないのです
が、$SAFE=3の時は動くけれど、$SAFE=4だと相変わらず失敗するようです。
(snip)
r33328 で直ったようですが、1.9.3に入れますか？みなさんの意見が聞きたいです。
正直ぼくのなかでは重要度は微妙なんですけど、このままだた1.9.[234] でそれぞれ
仕様が違うという事態になるので、それもいかがなものかと思い悩んでいます。

アプリケーション側のworkaroundとしては、$SAFE >= 4のサンドボックス内で
使用を許可したい変換表を$SAFE == 0のときにあらかじめロードしておくとい
う手があるので、tDiaryのケースでそれが現実的な解となるかどうかが、一つの
判断材料ですね。

$SAFEが4以上の時はグローバルな状態の変更が禁止されるという原則があった
と思うんですが、このケースはそれに抵触しないのかなというのがちょっと気に
なります。影響は限定的なので実害はない気がするのですが。

1.9.[234]で仕様が違うという点については、どうせ$SAFEは2.0で削除すべきだ
と思うので、あんまり気にしなくてもいいんじゃないでしょうか。

--
Shugo Maeda

[kosaki (Motohiro KOSAKI)]

2011年9月27日2:19 Shugo Maeda [e-mail]:

前田です。

2011年9月26日22:26 KOSAKI Motohiro [e-mail]:

Yuguiさんが1.9.3ブランチにバックポートしてくださって、先ほどめでたく
1.9.3RC1が出た今ころになって気づいて、ものすごく申し訳ないのです
が、$SAFE=3の時は動くけれど、$SAFE=4だと相変わらず失敗するようです。
(snip)
r33328 で直ったようですが、1.9.3に入れますか？みなさんの意見が聞きたいです。
正直ぼくのなかでは重要度は微妙なんですけど、このままだた1.9.[234] でそれぞれ
仕様が違うという事態になるので、それもいかがなものかと思い悩んでいます。

アプリケーション側のworkaroundとしては、$SAFE >= 4のサンドボックス内で
使用を許可したい変換表を$SAFE == 0のときにあらかじめロードしておくとい
う手があるので、tDiaryのケースでそれが現実的な解となるかどうかが、一つの
判断材料ですね。

$SAFEが4以上の時はグローバルな状態の変更が禁止されるという原則があった
と思うんですが、このケースはそれに抵触しないのかなというのがちょっと気に
なります。影響は限定的なので実害はない気がするのですが。

1.9.[234]で仕様が違うという点については、どうせ$SAFEは2.0で削除すべきだ
と思うので、あんまり気にしなくてもいいんじゃないでしょうか。

なんと。そうだったのか。
話はそれてしまいますが、そろそろ2.0 (or 3.0) で消す予定のfeatureについて
w.r.o にwikiページつくりたいですね。
なんとなく、DLとsyckは代替ライブラリが出来たので消え行く運命なのかと思ってる。
ほかにも weakrefとか仕様レベルで問題があってメンテ不能なライブラリとか。

[yugui (Yuki Sonoda)]

重要度は確かに微妙です。微妙なので、もはや仕様は完全に凍結するという原則論を優先します。何か現実的なセキュリティーホールに繋がる可能性があったら(DoS以外)また考えます

[kazuhiko (Kazuhiko Shiozaki)]

かずひこです。

On 26/09/2011 19:19, Shugo Maeda wrote:

Yuguiさんが1.9.3ブランチにバックポートしてくださって、先ほどめでたく
1.9.3RC1が出た今ころになって気づいて、ものすごく申し訳ないのです
が、$SAFE=3の時は動くけれど、$SAFE=4だと相変わらず失敗するようです。
(snip)
r33328 で直ったようですが、1.9.3に入れますか？みなさんの意見が聞きたいです。
正直ぼくのなかでは重要度は微妙なんですけど、このままだた1.9.[234] でそれぞれ
仕様が違うという事態になるので、それもいかがなものかと思い悩んでいます。

アプリケーション側のworkaroundとしては、$SAFE >= 4のサンドボックス内で
使用を許可したい変換表を$SAFE == 0のときにあらかじめロードしておくとい
う手があるので、tDiaryのケースでそれが現実的な解となるかどうかが、一つの
判断材料ですね。

Yuguiさんの言うように、重要度は微妙というのには同意します。すくなくとも
tDiary的には、上記のワークアラウンドですでに回避できているので、まあ困ら
ないといえば困りません。

$SAFEが4以上の時はグローバルな状態の変更が禁止されるという原則があった
と思うんですが、このケースはそれに抵触しないのかなというのがちょっと気に
なります。影響は限定的なので実害はない気がするのですが。

この修正の結果「グローバルな状態が変更される」のは、あくまでも（現状の）
実装の都合による副作用であって、それが理由でこの修正をrevertすることは無
いんじゃないかなと思います。
それはそれとして、1.9.3に対しては、タイミングが遅すぎたと思うので、「も
はや仕様は完全に凍結するという原則論を優先します」というYuguiさんの判断
に異論はありません。

1.9.[234]で仕様が違うという点については、どうせ$SAFEは2.0で削除すべきだ
と思うので、あんまり気にしなくてもいいんじゃないでしょうか。

レンタルtDiaryの運営者としては、このissueそのものよりも↑こっちの方が気に
なります。:)

かずひこ

[shugo (Shugo Maeda)]

前田です。

2011年9月28日21:15 Kazuhiko [e-mail]:

1.9.[234]で仕様が違うという点については、どうせ$SAFEは2.0で削除すべきだ
と思うので、あんまり気にしなくてもいいんじゃないでしょうか。

レンタルtDiaryの運営者としては、このissueそのものよりも↑こっちの方が気に
なります。:)

$SAFEは他の処理系でまったく実装されていないですし、サンドボックスとしても
中途半端なので、根本的に見直した方がよいと思っています。
が、合意が得られているわけではないので、結局2.0でも残ってしまうかもしれません。

tDiaryのプラグインについては、本来Rubyのコードをユーザに記述させる必要ない
と思うのですが、互換性的に記述方法を変えるのは難しいのでしょうね。
記法がある程度限定されていれば、自前でパースして、Rubyのコードとしてではなく
プラグイン処理だけ実行できるかもしれませんが…。

--
Shugo Maeda

[kazuhiko (Kazuhiko Shiozaki)]

On 28/09/2011 14:15, Kazuhiko wrote:

On 26/09/2011 19:19, Shugo Maeda wrote:

Yuguiさんが1.9.3ブランチにバックポートしてくださって、先ほどめでたく
1.9.3RC1が出た今ころになって気づいて、ものすごく申し訳ないのです
が、$SAFE=3の時は動くけれど、$SAFE=4だと相変わらず失敗するようです。
(snip)
r33328 で直ったようですが、1.9.3に入れますか？みなさんの意見が聞きたいです。
正直ぼくのなかでは重要度は微妙なんですけど、このままだた1.9.[234] でそれぞれ
仕様が違うという事態になるので、それもいかがなものかと思い悩んでいます。

アプリケーション側のworkaroundとしては、$SAFE >= 4のサンドボックス内で
使用を許可したい変換表を$SAFE == 0のときにあらかじめロードしておくとい
う手があるので、tDiaryのケースでそれが現実的な解となるかどうかが、一つの
判断材料ですね。

Yuguiさんの言うように、重要度は微妙というのには同意します。すくなくとも
tDiary的には、上記のワークアラウンドですでに回避できているので、まあ困ら
ないといえば困りません。

えーと、さらに調査の結果、encodingによっては上記のワークアラウンドでうま
く動かないことがわかりました。

例えば、

$ ruby1.9 -ve 'a="あ";"あ".encode("euc-jp"); $SAFE=4; a.encode("euc-jp")'
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]

↑これは動きますが、

$ ruby1.9 -ve 'a="あ";"".encode("euc-jp"); $SAFE=4; a.encode("euc-jp")'
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]
-e:1:in encode': Insecure operation - encode (SecurityError) from -e:1:in '

↑これは動きません。

この「あらかじめロードする」技が、対象となるStringと関係なく動くわけでは
ないとなれば、残念ながらワークアラウンドになりません。

なお、1.9.3RC1にr33328を適用すれば、もちろん無事に動きます。

というわけで、引き続き重要度が微妙なのは同意しますが、「レンタルtDiaryを
Ruby-1.9で動かしている数少ない管理人の一人」としては1.9.3に入ると嬉し
い、という感じです。

とりあえず運用環境では1.9.3RC1にr33328を適用して動かすことにしました。

かずひこ

[naruse (Yui NARUSE)]

2011年10月4日19:02 Kazuhiko [e-mail]:

えーと、さらに調査の結果、encodingによっては上記のワークアラウンドでうま
く動かないことがわかりました。

例えば、

$ ruby1.9 -ve 'a="あ";"あ".encode("euc-jp"); $SAFE=4; a.encode("euc-jp")'
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]

↑これは動きますが、

$ ruby1.9 -ve 'a="あ";"".encode("euc-jp"); $SAFE=4; a.encode("euc-jp")'
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]
-e:1:in encode': Insecure operation - encode (SecurityError) from -e:1:in '

↑これは動きません。

この「あらかじめロードする」技が、対象となるStringと関係なく動くわけでは
ないとなれば、残念ながらワークアラウンドになりません。

空文字や ASCII 文字列はエンコーディング変換を要しないので変換ライブラリをロードしません。
なので、空文字では workaround にはなりません。
"\uFEFF".encode(enc) rescue nil
などとすれば任意のエンコーディングでいけるはずです。

--
NARUSE, Yui [e-mail]

[kazuhiko (Kazuhiko Shiozaki)]

On 04/10/2011 12:22, NARUSE, Yui wrote:

例えば、

$ ruby1.9 -ve 'a="あ";"あ".encode("euc-jp"); $SAFE=4; a.encode("euc-jp")'
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]

↑これは動きますが、

$ ruby1.9 -ve 'a="あ";"".encode("euc-jp"); $SAFE=4; a.encode("euc-jp")'
ruby 1.9.3dev (2011-09-23 revision 33323) [x86_64-linux]
-e:1:in encode': Insecure operation - encode (SecurityError) from -e:1:in '

↑これは動きません。

この「あらかじめロードする」技が、対象となるStringと関係なく動くわけでは
ないとなれば、残念ながらワークアラウンドになりません。

空文字や ASCII 文字列はエンコーディング変換を要しないので変換ライブラリをロードしません。
なので、空文字では workaround にはなりません。
"\uFEFF".encode(enc) rescue nil
などとすれば任意のエンコーディングでいけるはずです。

おぉ、素晴らしい。無事に回避できました。ありがとうございます。

かずひこ