Bug #8716

segmation fault 正規表現で大量のグループを利用時

Added by taka-yoshi taka almost 2 years ago. Updated 9 months ago.

[ruby-dev:47562]
Status:Closed
Priority:Normal
Assignee:Yui NARUSE
ruby -v:trunk Backport:2.0.0: DONE, 2.1: DONE

Description

=begin
WindowsとOS Xで検証しました。
*再現手順 ruby 2.0.0p247 (2013-06-27) [x64-mingw32]
a="()"
(32767.times{a<<'()'}
eval "/#{a}/=~''"

*再現手順 ruby 2.0.0p0 (2013-02-24 revision 39474) [x86_64-darwin12.2.1]
a="()"
(1<<21).times{a<<'()'}
eval "/#{a}/=~''"

以上よろしくお願いします。
=end

Associated revisions

Revision 47223
Added by Tomoyuki Chikanaga 9 months ago

merge r46831 partially. extracted commits are as follows.
https://github.com/k-takata/Onigmo/commit/b9fba1dc63ccb42a86e934011b468e6022fabb74
https://github.com/k-takata/Onigmo/commit/c1fc76b9bd463948ffc5058bc352bf93732f0314
https://github.com/k-takata/Onigmo/commit/a0efc0a200f7108ca3d5ac3039c8f952e0051619
https://github.com/k-takata/Onigmo/commit/c7cda4ed5676167b0d01bb5555724f6164fbdb13
[Bug #8716]

  • include/ruby/oniguruma.h (ONIG_MAX_CAPTURE_GROUP_NUM,
    ONIGERR_TOO_MANY_CAPTURE_GROUPS): add cheking the number of capture
    groups.

  • regerror.c (onig_error_code_to_format): ditto.

  • regparse.c (scan_env_add_mem_entry): ditto.

  • regexec.c (onig_region_copy, match_at): fix: segmation fault occurs
    when many groups are used.

Revision 47400
Added by Usaku NAKAMURA 9 months ago

merge r46831 partially. these changes are from:
https://github.com/k-takata/Onigmo/commit/7abd7b29481f98eb92be786e3d33611fc7d000a0
[Bug #8716]

  • include/ruby/oniguruma.h (ONIG_MAX_CAPTURE_GROUP_NUM,
    ONIGERR_TOO_MANY_CAPTURE_GROUPS): add cheking the number of capture
    groups.

  • regerror.c (onig_error_code_to_format): ditto.

  • regparse.c (scan_env_add_mem_entry): ditto.

  • regexec.c (onig_region_copy, match_at): fix: segmation fault occurs
    when many groups are used.

History

#1 Updated by Tomoyuki Chikanaga almost 2 years ago

  • Project changed from Backport200 to Ruby trunk
  • Tracker changed from Backport to Bug
  • Status changed from Open to Feedback

Backport200 は 2.0.0 ブランチへのバックポートチケット用ですので一旦 ruby-trunk へ移動します。

なお手元では trunk でも 2.0.0 でも再現しませんでした。
SEGV 発生時のバックトレースなどのメッセージを添付していただけませんか。
Mac OS X 版では C のバックトレースは ~/Library/Logs/DiagnosticReports の下にファイルで置かれますので、そちらもお願いします。

#2 Updated by Nobuyoshi Nakada almost 2 years ago

  • Target version set to 2.1.0
  • Status changed from Feedback to Assigned
  • ruby -v set to trunk
  • Category set to regexp

regexec.c:match_at()で呼ばれるSTACK_INITがサイズを考慮せずにxallocaしているため、スタックオーバーフローしています。

#3 Updated by Nobuyoshi Nakada almost 2 years ago

  • Description updated (diff)
  • Assignee set to Ken Takata

#4 Updated by Ken Takata almost 2 years ago

採用予定のパッチです。
https://github.com/k-takata/Onigmo/commit/b9fba1dc63ccb42a86e934011b468e6022fabb74
Windowsで落ちなくなったことは確認しましたが、それ以外の環境は未確認です。

#5 Updated by Hiroshi SHIBATA over 1 year ago

  • Target version changed from 2.1.0 to current: 2.2.0

#6 Updated by Ken Takata about 1 year ago

https://github.com/k-takata/Onigmo/commit/b9fba1dc63ccb42a86e934011b468e6022fabb74

Rubyにはおそらく影響しないのですが、上記リンク先を見ていただければ分かるとおり、
マルチスレッド環境でこの変更がうまく動かない場合があるようなので、修正方法を再度検討中です。

#7 Updated by Ken Takata about 1 year ago

落ちる原因が2つあることが分かりました。

  1. alloca でスタックオーバーフロー (修正済み)
  2. そもそも鬼雲(鬼車も)は、グループの数を short で管理しているため、32767個以上のグループを管理できない。32768個あるとインデックスが負になり、不正アクセスが発生。(未修正)

グループの数を 32bit で管理するようにするか、それとも 16bit のままとし、不正アクセスが発生しないように何らかのチェックを入れるか検討が必要です。

#8 Updated by Ken Takata about 1 year ago

キャプチャグループの数を ONIG_MAX_CAPTURE_GROUP_NUM (32767) 個に制限することにしました。
それに合わせて、ONIGERR_TOO_MANY_CAPTURE_GROUPS というエラーコードを新設することにしました。
https://github.com/k-takata/Onigmo/commit/c7cda4ed5676167b0d01bb5555724f6164fbdb13
tmp/ruby-2.0.x ブランチも更新済みです。 # ブランチ名をそろそろどうにかすべきかも

#9 Updated by Ken Takata 10 months ago

  • Assignee changed from Ken Takata to Yui NARUSE

r46831 で Onigmo 5.14.1 がマージされましたので、これも取り込まれました。
チケットのクローズ手順がよく分かっていないので、処理をお願いします。>NARUSEさん
(あと #9344 も)

Ruby 2.0/2.1へのバックポートはどうしましょうか。

#10 Updated by Shota Fukumori 10 months ago

  • Status changed from Assigned to Closed

閉じます

Edit → Status → Closed ですね。あるいは ChangeLog/commitMessage で [Bug #8716] とか。
http://img.sorah.jp/20140717_16.34_urw8d_Bug_8716_segmation_fault___rubytrunk__Ruby_Issue_Tracking_System.png

#11 Updated by Usaku NAKAMURA 10 months ago

  • Backport set to 2.0.0: REQUIRED, 2.1: REQUIRED

Ken Takata wrote:

Ruby 2.0/2.1へのバックポートはどうしましょうか。

該当修正部分のみを抽出して取り込むことになるかと思います。

しかし、backportチケットから持ってくるとBackport欄が空白になるというのは
本当に困るのでどうにかならないものかしらん?

#12 Updated by Ken Takata 10 months ago

クローズの際の取り決めがあるのかということを気にしていました。
(勝手に閉じて良いのかとか、Backport 欄は誰が何を設定するとか。)

該当修正部分のみを抽出して取り込むことになるかと思います。

関連コミットを挙げておきます。以下の4件となります。

  1. alloca でスタックオーバーフロー
    https://github.com/k-takata/Onigmo/commit/b9fba1dc63ccb42a86e934011b468e6022fabb74
    https://github.com/k-takata/Onigmo/commit/c1fc76b9bd463948ffc5058bc352bf93732f0314
    https://github.com/k-takata/Onigmo/commit/a0efc0a200f7108ca3d5ac3039c8f952e0051619

  2. キャプチャグループの数を ONIG_MAX_CAPTURE_GROUP_NUM (32767) 個に制限
    https://github.com/k-takata/Onigmo/commit/c7cda4ed5676167b0d01bb5555724f6164fbdb13

1.9.3 もバックポート対象とした方が良いかもしれません。(が、コンフリクトが発生するはず。)

#13 Updated by Usaku NAKAMURA 10 months ago

Ken Takata wrote:

クローズの際の取り決めがあるのかということを気にしていました。
(勝手に閉じて良いのかとか、Backport 欄は誰が何を設定するとか。)

直したら閉じていいですし、直ってるのに閉じてなかったら閉じていいです。
……というくらいのゆるーい運用だと思います、現状は。
あまり厳密なルール作ってもどうせ守られないので、こんなもんでよろしいかと。

Backport欄はUNKNOWNで放置するのが安全です。
それなりの確信があればREQUIREDやDONTNEEDにしてもかまいません。
削除だけは大変に困るので避けていただけると幸いです。

該当修正部分のみを抽出して取り込むことになるかと思います。
関連コミットを挙げておきます。以下の4件となります。

ありがとうございます。助かります。

1.9.3 もバックポート対象とした方が良いかもしれません。(が、コンフリクトが発生するはず。)

1.9.3は通常メンテナンスフェーズを終えているので、セキュリティイシューしか取り扱いません。

#14 Updated by Tomoyuki Chikanaga 9 months ago

  • Backport changed from 2.0.0: REQUIRED, 2.1: REQUIRED to 2.0.0: REQUIRED, 2.1: DONE

r47223 で ruby_2_1 ブランチに r46831 から抽出して頂いた関連コミットの部分をバックポートしました。
関連する変更点を提示して頂いて大変助かりました、ありがとうございます。

#15 Updated by Usaku NAKAMURA 9 months ago

  • Backport changed from 2.0.0: REQUIRED, 2.1: DONE to 2.0.0: DONE, 2.1: DONE

遅ればせながら ruby_2_0_0 にも取り込みました。ありがとうございます。

Also available in: Atom PDF