[スレッド全体]

2016/11/30 (水) 01:15:39 ばぼ  
Mozilla/5.0 (Windows NT 6.3; WOW64; Trident/7.0; rv:11.0) like Gecko
[2386] Re4:セキュアなsnprintf関数。
> この二つは何がどう違うのか説明可能ですか。
> void my_functuin1(wchar* out, size_t size){
>     auto_snprintf_s(out, size, size, L"%d", g_linenum);
> }
> void my_functuin2(wchar* out, size_t size){
>     auto_snprintf(out, size, L"%d", g_linenum);
> }


・1はoutにsize - 1文字書き込んで'\0'を付加します(溢れません)。
 出力がsizeを越えた場合、出力はsize-1文字に切り詰められます(溢れません)。
 出力がsizeを越えた場合、バッファサイズが足りない警告(戻り値!=0)が出ます。
・2はoutにsize文字書き込んで'\0'を付加します(出力=sizeで溢れます)。
 出力がsizeを越えた場合、出力はsize-1文字に切り詰められます(溢れません)。


いちおうちゃんと説明しましょう。

何が問題なのかを知るためには
sprintfシリーズの関数について、
発生した経緯から語るのがよさそうです。

まず、sprintf関数について。
40年くらい前、C言語が発生すると同時に生まれました。

 sprintf(out, format, ...)
 outに...の内容をformatの書式で出力する。

任意のデータを書式出力するprintf系関数の1つです。
出力先がメモリなのでI/O処理が要らず高速です。
確保したメモリの領域外に出力があふれてしまう障害、
バッファオーバーフローの危険が古くから指摘される関数です。

つぎに、snprintf関数。
15年くらい前にC言語の規格が改定されるときにできました。
sprintfが抱えるオーバーフロー問題への対策版です。

 snprintf(out, count, format, ...)
 countで最大文字数を指定し、出力を制限できる。

count文字出力すると末尾に'\0'を付加して終了します。
出力数がcount+1になるのが微妙ですが、
これはC言語が規格として提供するセキュア版sprintfです。

最後にマイクロソフトのセキュア版関数について。
10年くらい前、vc++2005が出たときにできました。
ほとんどすべてのメモリ書込みを行う関数に_s付きバージョンができました。

 sprintf_s(out, size, format, ...)

size - 1文字出力すると末尾に'\0'を付加して終了します。
最大出力数はsizeになります。
sprintf_s関数を使えば、sprintfの抱える問題をカバーできます。


じゃあ、snprintf_s関数は?

 snprintf_s(out, size, count, format, ...)

・・・ぶっちゃけるとわたしにもよく分りません。

countには _TRUNCATE という特別な値を指定できるのですが、
_TRUNCATEを指定した場合の動作はsprintf_sと等価です。
size - 1文字を上限に、できるだけたくさん出力する、という動きになります。
なんで存在するのかよく分らない関数です。

いま、snprintf_sが定義されていて、
snprintfがない状態になっています。
当然ですが、sprintf_sはちゃんとあります。

必要性が微妙な関数が間違った定義で定義されていて、
本来の使い方をできない状態になっています。
(sprintf_sを使うべき箇所に誤ったsnprintf_s利用)

LR4氏の言うように「もっと大事なことがある」と思います。
ただ、他の部分の共通関数の品質は高いので、放置したままにしとくのももったいないなーとも思います。

2016/11/30 (水) 16:30:13 もか  
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
[2387] Re5:セキュアなsnprintf関数。
>sprintf_s(out, size, format, ...)
>size - 1文字出力すると末尾に'\0'を付加して終了します。


>countには _TRUNCATE という特別な値を指定できるのですが、
>_TRUNCATEを指定した場合の動作はsprintf_sと等価です。

言っている内容がいまいち理解できないのは、たぶんこの前提が間違ってるからだと思いました。

char test[4];
int ret = _snprintf_s(test, 4, _TRUNCATE, "%s", "1234567890");
>ret=-1 test="123"
int ret = _snprintf_s(test, 4, 3, "%s", "1234567890");
>ret=-1 test="123"
int ret = _snprintf_s(test, 4, 4, "%s", "1234567890");
>実行時エラー
int ret = sprintf_s(test, 4, "%s", "1234567890");
>実行時エラー
こうなるみたいです。
あとsprintf_sはVC2005では存在していなくて「_sprintf_s」ですよね。

>snprintfがない状態になっています。
参考 https://sourceforge.net/p/sakura-editor/patchunicode/82/
>どう倒しても紛らわしいので、
>・VCの _snprintf に対応する auto関数は提供しない。
>・今の auto_snprintf は auto_snprintf_s に変名。

と書かれています。

auto_snprintf_s→サクラでは_TRUNCATE前提でバッファ切り詰めで使う&-1
auto_sprintf_s→あふれたら異常終了
auto_sprintf→あふれないことが保証されたときに使ってもいい
どれも、引数のバッファ長は\0分を含める実装となっていると。

2016/11/30 (水) 16:31:39 もか  
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
[2388] Re5:セキュアなsnprintf関数。
それ以前の問題として、サクラの定義をよく見てみると、数か所間違いがあるようにみえます。
defineのほうはrev:3680以降からです。
こうじゃないでしょうか。

Index: sakura_core/util/string_ex.h
===================================================================
--- sakura_core/util/string_ex.h	(リビジョン 4157)
+++ sakura_core/util/string_ex.h	(作業コピー)
@@ -253,9 +253,9 @@
 
 //印字系
 #if defined(_MSC_VER) && _MSC_VER>=1400
-#define auto_snprintf_s(buf, count, format, ...) tchar_sprintf_s((buf), count, (format), __VA_ARGS__)
+#define auto_snprintf_s(buf, count, format, ...) tchar_snprintf_s((buf), count, (format), __VA_ARGS__)
 #define auto_sprintf(buf, format, ...)           tchar_sprintf((buf), (format), __VA_ARGS__)
-#define auto_sprintf_s(buf, nBufCount, format, ...) tchar_snprintf_s((buf), nBufCount, (format), __VA_ARGS__)
+#define auto_sprintf_s(buf, nBufCount, format, ...) tchar_sprintf_s((buf), nBufCount, (format), __VA_ARGS__)
 #else
(すみません省略します)
Index: sakura_core/util/tchar_printf.cpp
===================================================================
--- sakura_core/util/tchar_printf.cpp	(リビジョン 4157)
+++ sakura_core/util/tchar_printf.cpp	(作業コピー)
@@ -359,6 +359,7 @@
 
 
 // sprintf_sラップ
+// ※nBufCountをはみ出す場合は異常終了します。
 //
 // (実装について)
 //     内容が同じなので、templateでも良かったのですが、
@@ -418,7 +419,7 @@
 {
 	va_list v;
 	va_start(v,format);
-	int ret=tchar_vsprintf_s(buf,count,format,v);
+	int ret=tchar_vsnprintf_s(buf,count,format,v);
 	va_end(v);
 	return ret;
 }
@@ -426,7 +427,7 @@
 {
 	va_list v;
 	va_start(v,format);
-	int ret=tchar_vsprintf_s(buf,count,format,v);
+	int ret=tchar_vsnprintf_s(buf,count,format,v);
 	va_end(v);
 	return ret;
 }

2016/12/3 (土) 00:36:13 ばぼ  
Mozilla/5.0 (Windows NT 6.3; WOW64; Trident/7.0; rv:11.0) like Gecko
[2390] Re6:セキュアなsnprintf関数。
▼ もかさん
> それ以前の問題として、サクラの定義をよく見てみると、数か所間違いがあるようにみえます。
> defineのほうはrev:3680以降からです。

やっと言ってる意味が分かりました。
わたしの修正コードでなくて、
現在のコードに対する修正提案なんですね。


(tchar_snprintf_sの実装部分)
> -	int ret=tchar_vsprintf_s(buf,count,format,v);
> +	int ret=tchar_vsnprintf_s(buf,count,format,v);

コア実装がいまのままなら同じことなんで
間違っていても機能的には影響ないと思います。

機能的に影響がなければ直さない、
それが乙女のポリシー。

と言われたらもう何も言えねっす。


ちなみに、tchar_snprintf_sは直呼びじゃなく
auto_snprintf_sを経由するのが正道と思われます。

string_ex.h 260行目
inline int auto_snprintf_s(ACHAR* buf, size_t count, const ACHAR* format, ...)   { va_list v; va_start(v,format); int ret=tchar_vsnprintf_s(buf,count,format,v); va_end(v); return ret; }
inline int auto_snprintf_s(WCHAR* buf, size_t count, const WCHAR* format, ...)   { va_list v; va_start(v,format); int ret=tchar_vsnprintf_s(buf,count,format,v); va_end(v); return ret; }
inline int auto_sprintf(ACHAR* buf, const ACHAR* format, ...)                    { va_list v; va_start(v,format); int ret=tchar_vsprintf(buf,format,v); va_end(v); return ret; }
inline int auto_sprintf(WCHAR* buf, const WCHAR* format, ...)                    { va_list v; va_start(v,format); int ret=tchar_vsprintf(buf,format,v); va_end(v); return ret; }
inline int auto_sprintf_s(ACHAR* buf, size_t nBufCount, const ACHAR* format, ...){ va_list v; va_start(v,format); int ret=tchar_vsprintf_s(buf,nBufCount,format,v); va_end(v); return ret; }
inline int auto_sprintf_s(WCHAR* buf, size_t nBufCount, const WCHAR* format, ...){ va_list v; va_start(v,format); int ret=tchar_vsprintf_s(buf,nBufCount,format,v); va_end(v); return ret; }

こっちは合ってますな。

内部実装のva_arg版関数を呼び出すだけの代物なので、
呼び出し口で1か所対応すれば目的を果たせるはずです。
邪道を利用してる箇所は非常に少なかったはずなので、
呼出し側を修正して関数定義ごと削除するのが適切かと思います。

関数定義ごと削除してしまえば
誤って利用してしまうことの再発防止にもなりますし。

また、全く同じ意味の実体が別定義で存在するので
機能を損なうこともありません。


まぁ、機能的影響はゼロなので、
どうでもいいっちゃどうでもいい話。

[▼次のスレッド]
INCM/CMT
Cyclamen v3.81
[ut:0.010][st:0.000]