マクロの文字列型引数における文字列長が未チェックのものがあるのを修正したい

[usagisita]

問題内容

マクロ引数のうち文字列型を扱うものの一部で、ファイルパスなど内部仕様による制限が存在しているにもかかわらず、一切チェックされずバッファがコピーされたり強制終了したりする現状の動作を改善したいです。

再現手順

Editor.FileSaveAs("＜260文字を超えるような長い文字列＞")

などのマクロを実行します。

またExtHtmlHelpを悪用することで設定によってはwcscpyのバッファオーバーランによりメモリー上の共有データ構造体を破壊することができます。

なおInsText、InsBoxText、AddTailなどで巨大な文字列を指定しメモリー上限に達してクラッシュするようなタイプはこのIssueの考慮対象外としたいです。

再現頻度

いつも

問題のカテゴリ

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

その他

参考情報として、FileSaveAsマクロ等でのファイル名に関して、CON、PRN、NUL、AUX、COM1などの特殊ファイル名を指定すると、保存処理の段階でクラッシュしたり応答なしになったりすることもありますが、これも今回のIssueの範囲外としたいです。 対応を考えるのであれば、またIssueなどを作成するなりコメントなどをお願いします。

マクロの文字列長には「\0(NUL)までの長さ(wcslen、lstrlen)」と「文字列全体の長さ＝＝BSTRの長さ」が存在しています。 処理の仕方によっては、この2つの混同により現状では正しく処理されていないケースもあるかもしれません。 例えばInsText、InsBoxTextなどでは文字列の途中にNULが含まれるケースが想定されておりBSTRの長さが使わています。 一方、SearchNext、Replaceなど検索系ではNULまでの長さが使われているようです。NUL文字を検索するには正規表現でのエスケープ処理が必要です。 必要があるのであれば、これらの処置の確認もしたほうがいいかもしれません。

[usagisita]

文字列を引数に取る関数 対策が必要そうな関数の一覧暫定版 FileOpen BSTR I4 I4 BSTR FileSaveAsDialog BSTR I4 I4 FileSaveAs BSTR I4 I4 PutFile BSTR I4 I4 InsFile BSTR I4 I4 Diff BSTR I4 ExecExternalMacro BSTR BSTR ExecCommand BSTR I4 BSTR ExtHtmlHelp BSTR BSTR ExpandParameter BSTR FileOpenDialog BSTR BSTR FileSaveDialog BSTR BSTR FolderDialog BSTR BSTR

対策が必要なさそうな関数の一覧暫定版 InsBoxText BSTR InsText BSTR AddTail BSTR SearchNext BSTR I4 SearchPrev BSTR I4 Replace BSTR BSTR I4 ReplaceAll BSTR BSTR I4 Grep BSTR BSTR BSTR I4 GrepReplace BSTR BSTR BSTR BSTR KeywordTagJump BSTR BookmarkPattern BSTR I4 SetMsgQuoteStr BSTR TraceOut BSTR I4 StatusMsg BSTR I4 IsCurTypeExt BSTR IsSameTypeExt BSTR BSTR InputBox BSTR BSTR I4 MessageBox BSTR I4 ErrorMsg BSTR WarnMsg BSTR InfoMsg BSTR OkCancelBox BSTR YesNoBox BSTR CompareVersion BSTR BSTR GetCookie BSTR BSTR GetCookieDefault BSTR BSTR BSTR SetCookie BSTR BSTR BSTR DeleteCookie BSTR BSTR GetCookieNames BSTR GetStrWidth BSTR I4 GetStrLayoutLength BSTR I4 IsIncludeClipboardFormat BSTR GetClipboardByFormat BSTR I4 I4 SetClipboardByFormat BSTR BSTR I4 I4 CreateMenu I4 BSTR

[berryzplus]

大変難しい話題なので放置していました。

「問題内容」に対して「ん？」と思った点が3つありました。

マクロ引数のうち文字列型を扱うものの一部で、ファイルパスなど内部仕様による制限が存在しているにもかかわらず、一切チェックされずバッファがコピーされたり強制終了したりする現状の動作を改善したいです。

1. マクロ引数のうち文字列型を扱うもの BSTR型のマクロ引数は、「文字列」か「バッファ」のどちらかを想定している認識です。 C/C++での「文字列」とは NULで終端される「文字」のシーケンス です。 「バッファ」とはメモリー上のアドレス範囲を指す用語です。 サクラエディタはバイナリファイルを編集できることになってるので、InsTextなどのドキュメントデータ挿入系関数には「バッファ」が渡される構えがあったほうが良いと思います。
2. ファイルパスなど内部仕様による制限が存在している 内部仕様には「意図的に仕様化してあるもの」と「既存実装を追認したもの」がある認識です。 ファイルパスの260文字制約は後者で、CreateFileAがMAX_PATHを超えるパスを扱えないことに起因した制約です。 issueの雰囲気から「内部仕様に合わせてチェック入れようぜ！」が結論に見えるんですが、「内部仕様＝正しい」ではなさそうなので入れようとするチェックによっては「ん？」になる気がします。
3. 一切チェックされずバッファがコピーされたり強制終了したりする チェックしてエラーになったらどうするか？を決める（＝レビュアーに理解してもらう）のが大変そうです。 どうするか？の選択肢を2つ以上思いつかないので、決められないんじゃないかと思います。 選択肢＝コピーする前にメッセージボックスを出して中断し、関数呼出を失敗させる。

意図的に放置していたわけですが、3週間誰も反応してないことが対応の難しさを物語っている気がします。