脆弱性対策: 再帰処理（スタートツリー・パッケージの呼び出し）の限度数を下げる （無限ループに対する例外処理）
Issue #327
resolved
スタートツリー・パッケージの呼び出し・コールコンテントに上限数が無いと思われる現在の仕様は、あまりにも危険だと思います。 他のエンジンにも云える事かもしれませんが、一歩間違えれば出口のない無限ループが待っています。
悪意を持って作成されたシナリオはもちろんのこと、 ちょっとしたミスでも無限ループが簡単に発生することがあり、 容易にユーザのパソコンへ余計な負荷を与えることができてしまいます。 また、無限ループ内にあるコンテントによっては それを演出（ウェイト）と勘違いしてしまう可能性まであります。
そこで呼び出し・コール処理に上限数を設け、 異常が発生したことをユーザへ通知するのはどうでしょうか。 １千～１万回など、通常ではまずありえない（そこまで使用されない）と思われる数を 上限にすれば問題にはならないと思います。
Comments (24)
-
reporter
-
repo owner
たしかにこの問題は対策していますが、イベント処理のパフォーマンスとの兼ね合いで1000コンテント処理するたびに1回だけ入力を処理するような仕様になっており、プレイヤーの入力(例えばF9)がなかなか受け付けられないという事はありえます(私の経験では結構連打が必要です)。また、イベント処理は基本的に全速力で行うので、負荷の問題は解決しません。
イベントコンテント処理のカウントの仕方に問題があると、呼び出しやエリア移動などの組み合わせ次第で本当に操作不能になる可能性もあります。そうなるとバグなので、そのようなイベントができたら教えていただけると助かります。
それにしても限界の10億回は多いような……。
たしか技術的な本当の限界は20億回くらいだったと思うのですが、現実的には万単位でもまずたどり着きませんし、メモリ消費の問題などもあるので、様子を見て減らしてもいいかもしれませんね。
-
reporter
実際に無限再帰を何度か起こして問題を自力で解決・理解されたか、 プログラム関係に深く携わったことのある方にしかピンと来ない問題だと思い、 問題提起させていただきました。 （CardWirthはコール・呼び出しを多用する設計・構造（限界回数が多いほど好ましい）ですし。）
イベントコンテント処理のカウントの仕方に問題があると、呼び出しやエリア移動などの組み合わせ次第で本当に操作不能になる可能性もあります。そうなるとバグなので、そのようなイベントができたら教えていただけると助かります。
今回、無限再帰を起こしたときはウィンドウ右上の
×ボタンで終了したので、 完全に操作不能になるような自体かどうかは確認していません。 （問題が問題なので、なるべくなら検証すら避けたい事柄です……。）また、イベント処理は基本的に全速力で行うので、負荷の問題は解決しません。
説明が難しいですが、イベントの処理速度ではなく、 無限再帰に陥ってから解決するまでにかかる負荷が問題だと思いました。
F9やAlt+F4などで迅速に対処できる（異変に素早く気付ける）ユーザならともかく、 それ以外のユーザでは遥か先の限界回数に達するまで延々と 負荷を与え続けることになってしまうことが考えられます（特にウェイト無し）。 また、セリフなどが挟まれるならともかくフラグ関係のみの処理では異常に気付きにくいと思います。私自身、後者（フラグ処理）の無限再帰が発生してもすぐに気付く事ができたので 重要度や危険性はまあ低いものだとは思いますが……。
ただこれは危ないな、と思っただけですので。 @k4nagatsukiさんのご判断に従います。
-
repo owner
1つのプログラミングが計算資源を使い果たしてしまう問題については、今時の大抵は2コア以上あるCPU、高負荷下でもなんとか操作可能なOSなど、状況はよくなっていると思いますが、問題がなくなったわけではありません。
問題を回避するには、例えばある程度イベントコンテントの処理を行ったら自律的に処理速度を落として負荷を下げる(たぶん1.30以前のCWがやっていた)とか、無限ループをなんとかして検出して処理を中断するといったやり方が考えられますが、今の状況ではどちらも現実的ではありません。高負荷無限処理は再帰を用いずループだけで作れてしまいますし、状況はいくらでも複雑にできます。
少なくともOSを巻き込んで操作不能にするような状態は発生しないのではないかと思っており、それさえ回避できればユーザは発生した問題から離脱可能なのであって、その辺りが妥協点ではないかと考えているのですが……。
-
repo owner
手許で少し実験してみましたが、再帰数が100,000くらいになると、再帰しすぎエラーを発生させた後で呼び出し構造からの脱出に時間がかかりはじめるようです(その間は操作不能となり、無理やり終了しようとすると本物のエラーになります)。
現実の処理では、数百層の再帰でもほぼありえないと想定してよさそうに思えます。まして数千・数万はないでしょう。とりあえず10,000層辺りを限度にしてみるのがよいかもしれません。
-
reporter
わざわざ実験までしてくださり、ありがとうございます。
さすがに「万単位じゃ足りない」という意見が出される事は無いとは思いますが、 数百など上限が低めでも問題になる可能性もあります。 多く見積もっても その辺り（10,000層）が無難だと私も思います。
設計などが大きく異なるので あまり参考にはならないと思いますが、 エンターブレイン社のRPGツクールシリーズの再帰の限度は1,000層だそうです。 RPGツクール2000でも再帰処理だけに安全装置を組み込み、 ただのループはやはり対策していなかったはずです。 （安全性には欠けますが、ユーザとしてもその方が使い勝手が良かったですね。あれは並列処理（バックグラウンド）も使えたので。）
-
reporter
- changed title to 脆弱性対策: 再帰処理（スタートツリー・パッケージの呼び出し）の限度数を下げる （無限ループに対する例外処理）
- marked as minor
Issueの表題がまるで、今まで対策していなかったように見えてしまっていたので、 「上限数を設ける」から「限度数を下げる」へ変更しました。
-
repo owner
少し技術的な話を。
ループと再帰は問題の性質が違うので、ツクールでもループに対してはなにもせず、再帰にはそのような制限をかけているのだと思います。
具体的には、リンクによる単純なループ処理などは、処理本体の他にコストを支払わずに実行できますが、コールは「呼び出し元を覚えておく」というコストがかかります。繰り返しコールを行うと「呼び出し元」情報の長い行列ができ、しかも処理終了後にはその行列を逆に辿って次々と戻っていく作業が発生します。そのため、コールはリンクと比べて遥かに高コストです。これはCWやツクールに限らずプログラム一般の話です(ちなみにCWのイベントデータは1コンテントごとにコール的なものが発生する構造で、最大の設計ミスと言えるほど筋が悪いので、そのうちなんとかしようと考えています)。
さて、コールというのは、よほど変なことをしないかぎり、本質的にそこまで階層が深くなるものではありません。処理末尾のコールをリンクへ置換するなどすれば、普通は5～10階層もあれば充分です。数千階層もの呼び出し構造が発生する処理は明らかに設計ミスをしています。ツクールはその辺りを前提にして、さらに相当な余裕を持たせて1000階層としているのだと思います。CWでもそれくらいあれば明らかに充分すぎますが、実際の処理は10000階層でも余裕がありそうなので、それくらいにしておこうかと思います。
-
reporter
先に負荷の話が出ていたため、つい高負荷無限処理（ループ）と再帰を混ぜて考えてしまいました。 元の表題の件といい、大変失礼しました。 しかしまさか、そんなまた別の問題があったとは驚きました……。
あまりこういった事ははっきりと言いたくはなかったのですが、 初歩的な設計ミス（処理末尾にコール）や 多重に再帰を用いた設計（例えば判定の判定の判定などといった形で数十階層以上使用）は、 よほど不慣れな方か変態のどちらかだと、私もそう思います。 しかし、誰しもそんなに上手くきれいに見事なシナリオを組み上げる事が出来る訳ではありませんので、 少々トボケた発言をしてしまいました。
10,000階層は処理速度が鈍ることのない問題のない範囲のようですし、 上限数に大きく余裕を持たせることに賛成します。
-
repo owner
呼び出し階層の話はプログラマでもなければ知っているような事ではありませんし、今回の変更で現実的な時間で出るようになった警告が、その問題の存在を知らせる役に立つとラッキーかなぁという感じですね。
というわけで、pull request #1322で実際に更新しました。
-
reporter
そうですね。私も少々大げさなところはあると自覚していますが、 こうも他の方のご意見が出されなかったことを考えると 少しさびしい感じもします。 （私事ですが実際、プログラム関係の方に口をすっぱくしてブレイク処理の有無やループ・再帰には細心の注意を払えと教えられたことがありました。）
それはさておき。
限度数が更新されましたので軽く回してみました。 警告を表示するだけなら一瞬でしたので、 テスト結果を報告させていただきます。条件は以下の通りです。
- エリア内のカードイベントにコール・リンクを１コンテント分だけ設置。
- パッケージ内の無限ループ処理にも、コール・リンク系コンテントのみを使用。
・パッケージ
- 同パッケージ内はツリー・パッケージのコールを問わず、どちらも警告を表示した。
- 同パッケージへのリンクの場合は、警告を表示した。
- 同パッケージ内のツリーAとBの交互コールの場合は、警告を表示した。
- パッケージAとBの交互コール・リンクの場合は、警告を表示した。
- 同パッケージ内のツリーへのリンクでは、通知は無い。（これは想定通りの動作であると判断しました。）
・エリア内カードイベント
- パッケージのリンク → 警告を表示した。想定通りに動作。
- パッケージのコール → 上記の警告の出るパターンでも通知無し。
- 同エリアカード内ツリーのコール → 通知無し。
- 同エリアカード内ツリーのリンク → 通知無し。（パッケージ同様、これについても想定通りの動作であると判断しました。）
限度数を引き下げるという、こちらの要求は達成されましたので 上記の検証結果に特に問題がないと ご判断されましたら、 @k4nagatsukiさんに当Issueのステータスを resolved へ変更していただければと思います。
-
意見が出せなくてすみません。とくに良い案が思い浮かばなかったことと上限数を処理速度に問題が発生しない範囲ならいくらでも良かったということ、
そして、プログラム作者ではないので中身に明るくなく上手く発言出来なかった次第です。 -
自分も無限ループを起こすと操作不能になるのは体験していたのですが、 まあ起こさなければいいだろうと言うのと、その対策で速度が低下したら困るなぁぐらいのことしか考えてませんでした… 丁度いい具合になったようでよかったです。
-
repo owner
人にはそれぞれ興味のある・詳しい分野とそうでもない分野がありますから、そうでない所には無理をして首を突っ込む必要はないと思いますよ。個人的な希望ではなんにでも突っ込んで全体を把握してくれる人がいるとありがたいなぁとは思いますが、CWPyの開発は仕事でなく余暇でする事ですので、どうか自分の好みのペースでお付き合いいただければと思います。
ところで、リンクで警告が出るというのはおかしいです。スタートのコールが絡まない限り、パッケージへのリンクは現在実行中のイベントをそのまま置換すれば階層を増やさずに済むのですが、その辺りの判断にバグがありそうです。ちょっと調べてみます。
-
repo owner
pull request #1328
条件式のミスで、パッケージへのリンクがコール状態になっていました。別にそのままでも正しく動きますが、呼び出し階層が増えていくのは問題です。今回階層を1万に制限したので現実的な問題にもなりえます。という事で修正しました。
イベントのコール・リンクに関わる修正を行うと、それらが正しく動かなくなる可能性が生じます。何か問題が発生したらお知らせください。
-
reporter
- attached 無限再帰テスト01.zip
cardwirthpy_20160213c CardWirthPy 0.12.4 Alpha 4 Build: 2016-02-13 12:25:50どうにも、再帰回数の数え方に問題があるように思います。
エリアにあるカード・パッケージを問わず、 無限再帰に陥るコールコンテント（呼び出し）の後に何か別のコンテントが続いていないと 警告が出されることはありませんでした。
- 検証にはフラグ反転コンテントを使用しました。
- 特定条件下でエンジン側がコールをリンクに置き換える処理を行っている場合、この報告はまったくの見当外れになります。
また、こちらは少々判断に困るのですが、 パッケージとエリアカードではコールの上限数が変わりました。 パッケージはエリアカードの半分でした。
-
reporter
上記の報告ですが、検証のために新規作成したシナリオを添付しました。 （うまく添付できているといいですが……。）
-
repo owner
検証シナリオまで作っていただきありがとうございます。
上の方にもちらっと書きましたが、コールは処理の末尾にある場合に限り、リンクへ置換する事ができます。それによって呼び出し階層を節約する事ができるため、可能であれば行うようにしています。
パッケージのコールが呼び出し階層を2倍消費してしまうのは、スタートのコールが元のコールコンテントの位置を覚えておけばいいのに対し、パッケージのコールは元のイベントツリーの場所に加えてそのイベントツリー内のコールコンテントの場所を覚えておかなければならないからです。
これを単数で数えるようにするのも理論的には可能ですが(イベントツリーの場所とコールコンテントの場所を一体化させて覚えればいい)、イベント処理の根幹部分を触る事になってしまうので、正直に言ってリスクを考えるとやりたくないです……。
-
reporter
＞ @k4nagatsukiさん
やはり想定通りの動作（置換処理）でしたか。問題がないようで安心しました。
パッケージの階層食いについては、無闇にリスクを負うべきではないと私も思います。 @k4nagatsukiさんの仰った再帰10万回までは処理速度に問題は無いようですし、 やはり5,000～10,000回以上も再帰処理を行う方も（多分）現れることはないでしょう。 仮に何かあれば限度数を引き上げれば済む事であり、 追わずに済むリスクですので、現状はこのままで問題ないと思います。
＞ @crowstarさん、暗黒騎士（@akkw）さん
どこか非難するような言い方をしてしまい、申し訳なく思います。
（余談ですが ここ数ヶ月の間、 暗黒騎士さんのIDを打ち込んでもなぜか変換されません。 間違ってはいないはずなのですが……。またサイトの不調でしょうか。）
-
repo owner
私もたまに名前のリンクが張られていないのは気になっていましたが暗黒騎士さんだけでしたか。なぜなんだろう。
Issueのリンクが張られない問題も、いつの間にか書式が変わったのかと思ってマニュアルを見たりしているのですが、別にそんな気配もなかったりします。
-
非難とは受け取ってませんのでお気になさらず。自分も時々きつい言い方になっているかもしれませんが、国語が下手なだけですのでどうかご容赦下さい。
明らかに英語圏のサイトですし、やっぱり日本語は不味いんでしょうか…ちょっと変えて見ますね。
-
repo owner
あ、リンクがついた。そういうことか……。
このサイト、国際化はβですし、この問題は運営側に報告した方がいいんでしょうね。英語で。
読み書きくらいはちゃんと教わっておくべきだった。
-
パッケージのコールが呼び出し階層を2倍消費する件
現状のままで良いと思います。2倍消費してもまだまだ余裕があるのであれば下手にいじる必要はないかと
＞Liar_cwさん
非難というわけではないのでお気になさらないようにー。 -
repo owner
- changed status to resolved
すっかり忘れていましたが、表題の件を完了しているのでクローズします。
- Log in to comment
どうやらこれは既知の問題であり解決済みのようでした。 Issue
#117（pull request #169）実際に、シナリオ制作（テストプレイ）中に誤ってこの無限再帰（無限ループ）を起こしてしまったのですが、 軽度のフリーズによく似た状態、CPUへのそれなりの負荷、CWPy終了後にイベントビューアにエラーとして載ったりとしていたので 少々驚いてしまいました。
それにしても限界の10億回は多いような……。