Grepで別ウィンドウを開く際ごくまれにクラッシュする

usagisita commented 3 years ago

問題内容

CControlTray::OpenNewEditorでレスポンスファイルが使われるか使われないかの境界線で、1文字分バッファが不足しているため、クラッシュすることがあります。

再現手順

1000文字ぐらいの検索キーワードでGrepします。コマンドライン文字数が特定の文字列長になった場合のみクラッシュします。 sakura.exeのパス長などに依存するため特定の長さは環境により異なります。

再現頻度

特定条件下では100%

問題のカテゴリ

プログラムの動作上の問題
- 正式リリース版

環境情報

OS バージョン
サクラエディタバージョン
PC情報

スクリーンショット

https://github.com/sakura-editor/sakura/blob/10dda8eebc785423a778b2c14224497a6bbac93a/sakura_core/_main/CControlTray.cpp#L1164 分かりにくいですが、下のelse句での文字列を出力するときに入りきらないか判定しています。「1023の固定長(NULL除く)」＜「出力済みバッファ長(NULL除く) 」+ 「オプション文字列(NULL除く)」しかし実際の出力には「オプション文字列」だけでなく直前に「半角空白」が1文字出力されるため、1文字だけ足りません。 https://github.com/sakura-editor/sakura/blob/10dda8eebc785423a778b2c14224497a6bbac93a/sakura_core/_main/CControlTray.cpp#L1186 ここで1文字足りない場合はsprintf_s系のいつものセキュリティエラーでクラッシュします。

berryzplus commented 3 years ago

ここで1文字足りない場合はsprintf_s系のいつものセキュリティエラーでクラッシュします。

これ、要検証です。

CCommandLineString::AppendFの実装はこうなっています。 https://github.com/sakura-editor/sakura/blob/10dda8eebc785423a778b2c14224497a6bbac93a/sakura_core/basis/CMyString.h#L88-L94

最近ちょっとネタにしましたがサクラエディタの独自関数 auto_vsprintf_s の実態は snprintf_s(_TRUNCATE) なのでバッファが足りなくてもオーバーフローはしません。 https://github.com/sakura-editor/sakura/blob/10dda8eebc785423a778b2c14224497a6bbac93a/sakura_core/util/string_ex.h#L200-L201

まー「オーバーフローはしないはず」と思ってるだけで検証はしてないので、その辺りも含めて要検証だってことなんすけど。

usagisita commented 3 years ago

クラッシュの検証をしたのは数か月前以上だったので、最近の修正で最新masterでは_TRUNCATEになっている、という可能性は大いにあります。前から_TRUNCATEでしたっけ？ _TRUNCATEになるならなるで、出力内容がちょん切れて正しくなくて、実行時に問題になるはず可能性が高いので、直さなきゃならないという点では、一緒ではあります。その辺も含めて、ご考慮よろしくおねがいします。

berryzplus commented 3 years ago

クラッシュの検証をしたのは数か月前以上だったので、最近の修正で最新masterでは_TRUNCATEになっている、という可能性は大いにあります。前から_TRUNCATEでしたっけ？

ごく最近に tchar_printf を外した際に、tchar_printf が最終的にやってたことを貼り付けたのが先に挙げたコードです・・・が、実は元のコードがどうだったのかを改めて検証したわけではなく、だいぶ前に読み解いた内容を貼った感じでした。もしかすると、実体が snprintf_s(_TRUNCATE) だったのは vなしのほうだけだったのかも知れません。これに関しては、間違って揃えてしまってたとしても実害はないはずです。（落ちない方向に揃えてるから）

ここのエディタ起動の見直しは、何にしてもやりたいです。

sakura-editor / sakura