Bug #6861: ERB::Util.escape_html is not escaping single quotes

• Status: Closed
• Assignee: shugo (Shugo Maeda)
• Target version: -
• ruby -v: 2.0.0dev

[spastorino (Santiago Pastorino)]

We just fixed this issue in Rails
https://groups.google.com/forum/#!msg/rubyonrails-security/kKGNeMrnmiY/r2yM7xy-G48J%5B1-25%5D

Ruby's ERB is not escaping single quotes and this could lead to
security issues like ...

My Link!
being link = " '; alert(hax) "

OWASP suggest escaping &, <, >, ", ' and /
https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content

About / I don't think could lead to issues but that's another story.

You have the right code in CGI.escapeHTML
https://github.com/ruby/ruby/blob/c47cca2f/lib/cgi/util.rb#L36 so my
suggestion is to reuse CGI.escapeHTML from ERB::Util

I've sent a pull request https://github.com/ruby/ruby/pull/156

[shugo (Shugo Maeda)]

咳さん

前田です。

以下のチケットが登録されています。

HTMLではどの文字をエスケープすべきかは文脈によって異なるので、現状の動作は仕様
だと思うのですが、利便性を考えると修正してもよいのかなと思いますが、どうでしょう。

CGI.escapeHTMLの方はすでに修正されていますが、対応するなら'じゃなくて'
にすべきかなと思います。

2012/8/13 spastorino (Santiago Pastorino) [e-mail]:

Issue #6861 has been reported by spastorino (Santiago Pastorino).

---

Bug #6861: ERB::Util.escape_html is not escaping single quotes
https://bugs.ruby-lang.org/issues/6861

Author: spastorino (Santiago Pastorino)
Status: Open
Priority: Normal
Assignee:
Category:
Target version:
ruby -v: 2.0.0dev

We just fixed this issue in Rails
https://groups.google.com/forum/#!msg/rubyonrails-security/kKGNeMrnmiY/r2yM7xy-G48J%5B1-25%5D

Ruby's ERB is not escaping single quotes and this could lead to
security issues like ...

My Link!
being link = " '; alert(hax) "

OWASP suggest escaping &, <, >, ", ' and /
https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content

About / I don't think could lead to issues but that's another story.

You have the right code in CGI.escapeHTML
https://github.com/ruby/ruby/blob/c47cca2f/lib/cgi/util.rb#L36 so my
suggestion is to reuse CGI.escapeHTML from ERB::Util

I've sent a pull request https://github.com/ruby/ruby/pull/156

--
http://bugs.ruby-lang.org/

--
Shugo Maeda

咳といいます。

On 2012/08/13, at 10:30, Shugo Maeda [e-mail] wrote:

以下のチケットが登録されています。

HTMLではどの文字をエスケープすべきかは文脈によって異なるので、現状の動作は仕様
だと思うのですが、利便性を考えると修正してもよいのかなと思いますが、どうでしょう。

CGI.escapeHTMLの方はすでに修正されていますが、対応するなら'じゃなくて'
にすべきかなと思います。

githubのpull requestのコメントはちょっと見てました。

Rubyスクリプト的にはこんな感じで修正していいと思います。
私がわからないのは次の点でどなたか作業していただけませんか？

(a) エスケープすべき文字がこれで良いのか最近の事情がわからない
'っていうのは件のパッチの中にないけど「'」のことですか？

(a-1) testコードどうしよう

(b) github経由のコミットしたことなくてpull requestを受け入れる手順がわからない
この機会に覚えるべきという話も...

また、エスケープするサブルーチンをRubyで用意してそれをERBやCGIが呼び出すよう
なのはどうなんでしょうか。これは言語が備える機能ではないような気もするけど、
Cで書いてあったらなんだか速そう。

[shugo (Shugo Maeda)]

前田です。

2012年8月13日 10:59 Masatoshi SEKI [e-mail]:

Rubyスクリプト的にはこんな感じで修正していいと思います。
私がわからないのは次の点でどなたか作業していただけませんか？

とりあえず、私が引き取って対応しましょうか?

(a) エスケープすべき文字がこれで良いのか最近の事情がわからない
'っていうのは件のパッチの中にないけど「'」のことですか？

'の話は、CGI.escapeHTMLの方で'を'に変換しているという話です。
HTML4では'は使えないので数値文字参照にした方がよいのではないか
という話でした。

(a-1) testコードどうしよう

これは適当に追加しようと思います。

(b) github経由のコミットしたことなくてpull requestを受け入れる手順がわからない
この機会に覚えるべきという話も...

Ruby的にはSVNにcommitすればいいんじゃないですかね。

また、エスケープするサブルーチンをRubyで用意してそれをERBやCGIが呼び出すよう
なのはどうなんでしょうか。これは言語が備える機能ではないような気もするけど、
Cで書いてあったらなんだか速そう。

Cで書く必要はない気もしまが、CGIと同じコードがあるのはちょっと嫌ですねえ。
ERBの方でcgi/utilをrequireするのはまずいですか?

--
Shugo Maeda

[shugo (Shugo Maeda)]

前田です。

一点コメントし忘れてました。

2012年8月13日 11:38 Shugo Maeda [e-mail]:

(a) エスケープすべき文字がこれで良いのか最近の事情がわからない

<script>要素の中とかでは当然不十分だと思いますが、そういう特殊な 文脈以外ではこれくらいでいいのかなという気がします。 https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content だと/もエスケープすべきとしているんですが、必要ですかねえ? 単体で終了タグとみなされることはないのでがエスケープされて いれば十分な気がするのですが。 -- Shugo Maeda

[sorah (Sorah Fukumori)]

sora_h です

2012/8/13 Masatoshi SEKI [e-mail]:

(b) github経由のコミットしたことなくてpull requestを受け入れる手順がわからない
この機会に覚えるべきという話も...

github は svn.r-l.o からの一方通行なので github 経由でコミットはできないはずです。

pull request の URL 末尾に .diff をつければ生の diff ファイルが手に入るので、
patch -p1 < hoge.diff とかでパッチを適用すれば良いかと

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

[shyouhei (Shyouhei Urabe)]

On 08/13/2012 12:03 PM, Shota Fukumori (sora_h) wrote:

sora_h です

2012/8/13 Masatoshi SEKI [e-mail]:

(b) github経由のコミットしたことなくてpull requestを受け入れる手順がわからない
この機会に覚えるべきという話も...

github は svn.r-l.o からの一方通行なので github 経由でコミットはできないはずです。

仮に可能だとしても同期が外れるのでgit側からのコミットはおやめください。

このpull requestを採用してくれなどとお願いされれば卜部の側で手動で対応することはできるかと思います。

xibbarこと藤岡です。

(a) エスケープすべき文字がこれで良いのか最近の事情がわからない
'っていうのは件のパッチの中にないけど「'」のことですか？

'の話は、CGI.escapeHTMLの方で'を'に変換しているという話です。
HTML4では'は使えないので数値文字参照にした方がよいのではないか
という話でした。

これは ' が非推奨なのはセキュリティの観点じゃないんですか？

また、エスケープするサブルーチンをRubyで用意してそれをERBやCGIが呼び出すよう
なのはどうなんでしょうか。これは言語が備える機能ではないような気もするけど、
Cで書いてあったらなんだか速そう。

Cで書く必要はない気もしまが、CGIと同じコードがあるのはちょっと嫌ですねえ。
ERBの方でcgi/utilをrequireするのはまずいですか?

同じく、Cだったらrubygemsのライブラリで上書きしてもらって
速度を上げればいいんじゃないかと思っています。

ERB側がどう対応するかはおまかせで。

xibbarこと藤岡です。

2012年8月13日 11:38 Shugo Maeda [e-mail]:

(a) エスケープすべき文字がこれで良いのか最近の事情がわからない

<script>要素の中とかでは当然不十分だと思いますが、そういう特殊な 文脈以外ではこれくらいでいいのかなという気がします。 https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content だと/もエスケープすべきとしているんですが、必要ですかねえ? 単体で終了タグとみなされることはないのでがエスケープされて いれば十分な気がするのですが。

cgi.rbの方では / はエスケープするつもりなかったです。

' -> '
の数値への変更に関してはもう少々お待ち下さい。
これは入れるつもりです。

咳といいます。

On 2012/08/13, at 11:38, Shugo Maeda [e-mail] wrote:

前田です。

2012年8月13日 10:59 Masatoshi SEKI [e-mail]:

Rubyスクリプト的にはこんな感じで修正していいと思います。
私がわからないのは次の点でどなたか作業していただけませんか？

とりあえず、私が引き取って対応しましょうか?

ありがとうございます。おねがいします。

また、エスケープするサブルーチンをRubyで用意してそれをERBやCGIが呼び出すよう
なのはどうなんでしょうか。これは言語が備える機能ではないような気もするけど、
Cで書いてあったらなんだか速そう。

Cで書く必要はない気もしまが、CGIと同じコードがあるのはちょっと嫌ですねえ。
ERBの方でcgi/utilをrequireするのはまずいですか?

あ。ちょっと勘違いしてました。
CGI小さくなったんですね。

cgi/utilだけrequireして使えるならそれでもいいかなあ。

[shugo (Shugo Maeda)]

前田です。

2012年8月13日 12:35 fujioka [e-mail]:

(a) エスケープすべき文字がこれで良いのか最近の事情がわからない
'っていうのは件のパッチの中にないけど「'」のことですか？

'の話は、CGI.escapeHTMLの方で'を'に変換しているという話です。
HTML4では'は使えないので数値文字参照にした方がよいのではないか
という話でした。

これは ' が非推奨なのはセキュリティの観点じゃないんですか？

HTML4で定義されていないからだと理解しています。
XHTMLなら問題ないと思いますが、CGIやERBではXHTMLを仮定できない
のではないでしょうか。

--
Shugo Maeda

[shugo (Shugo Maeda)]

前田です。

2012年8月13日 12:47 Masatoshi SEKI [e-mail]:

2012年8月13日 10:59 Masatoshi SEKI [e-mail]:

Rubyスクリプト的にはこんな感じで修正していいと思います。
私がわからないのは次の点でどなたか作業していただけませんか？

とりあえず、私が引き取って対応しましょうか?

ありがとうございます。おねがいします。

了解しました。

また、エスケープするサブルーチンをRubyで用意してそれをERBやCGIが呼び出すよう
なのはどうなんでしょうか。これは言語が備える機能ではないような気もするけど、
Cで書いてあったらなんだか速そう。

Cで書く必要はない気もしまが、CGIと同じコードがあるのはちょっと嫌ですねえ。
ERBの方でcgi/utilをrequireするのはまずいですか?

あ。ちょっと勘違いしてました。
CGI小さくなったんですね。

そうですね、藤岡さんが分割してくださったので、こういう時に使いやすくなったと思います。

cgi/utilだけrequireして使えるならそれでもいいかなあ。

ではいったんその方向で対応します。何か問題があればお知らせください。

--
Shugo Maeda

[shugo (Shugo Maeda)]

• Assignee set to shugo (Shugo Maeda)

Hello,

Thanks for your report.

spastorino (Santiago Pastorino) wrote:

OWASP suggest escaping &, <, >, ", ' and /
https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content

About / I don't think could lead to issues but that's another story.

Agreed.

You have the right code in CGI.escapeHTML
https://github.com/ruby/ruby/blob/c47cca2f/lib/cgi/util.rb#L36 so my
suggestion is to reuse CGI.escapeHTML from ERB::Util

I and SEKI have discussed it, and have agreed to use cgi/util.
CGI.escapeHTML has a problem that is uses ' instead of ', but
xibbar will fix it later.

[shugo (Shugo Maeda)]

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

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

---

• lib/erb.rb (ERB::Util.html_escape): use CGI.escape to escape
  single quotes. [ruby-core:47138] [Bug #6861]

藤岡です。

2012年8月13日 12:35 fujioka [e-mail]:

(a) エスケープすべき文字がこれで良いのか最近の事情がわからない
'っていうのは件のパッチの中にないけど「'」のことですか？

'の話は、CGI.escapeHTMLの方で'を'に変換しているという話です。
HTML4では'は使えないので数値文字参照にした方がよいのではないか
という話でした。

これは ' が非推奨なのはセキュリティの観点じゃないんですか？

HTML4で定義されていないからだと理解しています。
XHTMLなら問題ないと思いますが、CGIやERBではXHTMLを仮定できない
のではないでしょうか。

escapeHTMLとはなんなのか？という問題に行き着くかと思います。
私も途中からcgiライブラリをメンテナンスしているので、
過去のことはちょっとわからないのですが、
escapeHTMLとかはHTMLの最新版を提供するものかなぁとか
勝手に思っていました。
なので、HTML5に全部合わせるタイミングでHTML4をバッツリ切って
やるのがいいのかなと思っていました。

' から ' へ変更するだけでいいんですかね。

• File noname noname added

On Mon, Aug 13, 2012 at 01:11:45PM +0900, shugo (Shugo Maeda) wrote:

Issue #6861 has been updated by shugo (Shugo Maeda).

Assignee set to shugo (Shugo Maeda)

Hello,

Thanks for your report.

spastorino (Santiago Pastorino) wrote:

OWASP suggest escaping &, <, >, ", ' and /
https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content

About / I don't think could lead to issues but that's another story.

Agreed.

You have the right code in CGI.escapeHTML
https://github.com/ruby/ruby/blob/c47cca2f/lib/cgi/util.rb#L36 so my
suggestion is to reuse CGI.escapeHTML from ERB::Util

I and SEKI have discussed it, and have agreed to use cgi/util.
CGI.escapeHTML has a problem that is uses ' instead of ', but
xibbar will fix it later.

Shouldn't CGI use ERB? It seems like ERB's use is for creating HTML,
where CGI is in charge of providing the common gateway interface.

ERB concerns itself with templating and should have knowledge of
template formats / escaping. It seems CGI would not.

--
Aaron Patterson
http://tenderlovemaking.com/

[spastorino (Santiago Pastorino)]

Shugo I've already sent an issue for the ' thing.

https://bugs.ruby-lang.org/issues/6850

and here https://github.com/ruby/ruby/pull/154 you have a pull request.

Thanks.

[shugo (Shugo Maeda)]

匿名ユーザ wrote:

escapeHTMLとはなんなのか？という問題に行き着くかと思います。
私も途中からcgiライブラリをメンテナンスしているので、
過去のことはちょっとわからないのですが、
escapeHTMLとかはHTMLの最新版を提供するものかなぁとか
勝手に思っていました。
なので、HTML5に全部合わせるタイミングでHTML4をバッツリ切って
やるのがいいのかなと思っていました。

cgi/htmlがHTML 3.2とHTML 4.01にしか対応していない現状では、むしろHTML4以前の
規格に合わせるべきではないでしょうか。
また、互換性の確保が難しくないのであれば(今回の件は難しくないと考えています)、
CGI.escapeHTMLのような基本的な機能はなるべく広く利用できた方がよいのではないか
と思います。

' から ' へ変更するだけでいいんですかね。

そのように理解しています。
https://github.com/ruby/ruby/pull/154 のパッチを適用していただくだけでよいかと思います。

[shugo (Shugo Maeda)]

Aaron Patterson wrote:

I and SEKI have discussed it, and have agreed to use cgi/util.
CGI.escapeHTML has a problem that is uses ' instead of ', but
xibbar will fix it later.

Shouldn't CGI use ERB? It seems like ERB's use is for creating HTML,
where CGI is in charge of providing the common gateway interface.

I admit that the name CGI is wrong. However, despite its name, CGI provides various features for Web applications. For example, cgi/html.rb provides features to generate HTML, and cgi/util.rb provides utility methods such as HTML.

ERB concerns itself with templating and should have knowledge of
template formats / escaping. It seems CGI would not.

HTML templating is the most common use case of ERB, but ERB is originally independent from HTML. For example, it can be used to embed Ruby code into TeX files.
Furthermore, ERB is provided as a single large file, and it's not a good idea to make CGI to depend the whole ERB.

[shugo (Shugo Maeda)]

spastorino (Santiago Pastorino) wrote:

Shugo I've already sent an issue for the ' thing.

https://bugs.ruby-lang.org/issues/6850

I have known the issue. Thank you.

and here https://github.com/ruby/ruby/pull/154 you have a pull request.

Thank you. I believe xibbar will take care of it.

[xibbar (Takeyuki FUJIOKA)]

cgi/htmlがHTML 3.2とHTML 4.01にしか対応していない現状では、むしろHTML4以前の
規格に合わせるべきではないでしょうか。
また、互換性の確保が難しくないのであれば(今回の件は難しくないと考えています)、
CGI.escapeHTMLのような基本的な機能はなるべく広く利用できた方がよいのではないか
と思います。

cgi/html は近いうちに HTML5 を入れる予定です。
そうなったときはescapeHTMLはどうしたら理想的だと思いますか？

[shugo (Shugo Maeda)]

前田です。

xibbar (Takeyuki Fujioka) wrote:

cgi/htmlがHTML 3.2とHTML 4.01にしか対応していない現状では、むしろHTML4以前の
規格に合わせるべきではないでしょうか。
また、互換性の確保が難しくないのであれば(今回の件は難しくないと考えています)、
CGI.escapeHTMLのような基本的な機能はなるべく広く利用できた方がよいのではないか
と思います。

cgi/html は近いうちに HTML5 を入れる予定です。
そうなったときはescapeHTMLはどうしたら理想的だと思いますか？

HTML5であればXHTMLでない時も'が使えるようですが、数値文字参照(')であればHTML4以前でも
XHTML/HTML5でも使えるので、数値文字参照にすべきだと思います。
Ruby 2.0では1.9との互換性を基本的に維持する方針ですので、cgi/htmlの方もHTML4対応のコードを
残すべきではないでしょうか。

[naruse (Yui NARUSE)]

shugo (Shugo Maeda) wrote:

xibbar (Takeyuki Fujioka) wrote:

cgi/htmlがHTML 3.2とHTML 4.01にしか対応していない現状では、むしろHTML4以前の
規格に合わせるべきではないでしょうか。
また、互換性の確保が難しくないのであれば(今回の件は難しくないと考えています)、
CGI.escapeHTMLのような基本的な機能はなるべく広く利用できた方がよいのではないか
と思います。

cgi/html は近いうちに HTML5 を入れる予定です。
そうなったときはescapeHTMLはどうしたら理想的だと思いますか？

HTML5であればXHTMLでない時も'が使えるようですが、数値文字参照(')であればHTML4以前でも
XHTML/HTML5でも使えるので、数値文字参照にすべきだと思います。
Ruby 2.0では1.9との互換性を基本的に維持する方針ですので、cgi/htmlの方もHTML4対応のコードを
残すべきではないでしょうか。

同感です。' の方がいいかな、1バイト短いし。

[shugo (Shugo Maeda)]

前田です。

naruse (Yui NARUSE) wrote:

同感です。' の方がいいかな、1バイト短いし。

https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content

にUsing hex entities is recommended in the specとあるのですが、これは
「文字実体参照に比べて」ということですかね。
10進よりも16進を推奨するような記述はHTML 4.01のspecには見当たりませんでしたが。

[naruse (Yui NARUSE)]

shugo (Shugo Maeda) wrote:

naruse (Yui NARUSE) wrote:

同感です。' の方がいいかな、1バイト短いし。

https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content

にUsing hex entities is recommended in the specとあるのですが、これは
「文字実体参照に比べて」ということですかね。
10進よりも16進を推奨するような記述はHTML 4.01のspecには見当たりませんでしたが。

だと思います。

[xibbar (Takeyuki FUJIOKA)]

では、1バイト減るということで、10進にしておきます。

• File noname noname added

On Tue, Aug 14, 2012 at 10:10:23AM +0900, shugo (Shugo Maeda) wrote:

Issue #6861 has been updated by shugo (Shugo Maeda).
Aaron Patterson wrote:

I and SEKI have discussed it, and have agreed to use cgi/util.
CGI.escapeHTML has a problem that is uses ' instead of ', but
xibbar will fix it later.

Shouldn't CGI use ERB? It seems like ERB's use is for creating HTML,
where CGI is in charge of providing the common gateway interface.

I admit that the name CGI is wrong. However, despite its name, CGI provides various features for Web applications. For example, cgi/html.rb provides features to generate HTML, and cgi/util.rb provides utility methods such as HTML.

OK! Then this change seems good.

ERB concerns itself with templating and should have knowledge of
template formats / escaping. It seems CGI would not.

HTML templating is the most common use case of ERB, but ERB is originally independent from HTML. For example, it can be used to embed Ruby code into TeX files.
Furthermore, ERB is provided as a single large file, and it's not a good idea to make CGI to depend the whole ERB.

Makes sense. Thanks! :-D

--
Aaron Patterson
http://tenderlovemaking.com/