fscanf(),scanf()とBuffer Overflow

質問

scanf("%s", buf);
で、bufの長さはどれくらいに取ればよいのでしょう。

sscanf(buf,"%s",buf2);
なら、sizeof(buf)以上に大きくならないでしょうが、scanf(), fscanf()で文字列を読み込むときは、Buffer Overflowの危険から逃れられないような気がしています。

私はそのため、文字列を扱うときには、この二つの関数を使わないでいるのですが、安全な使用方法はあるのでしょうか？

scanf("%10s",buf);
のような使い方は知っています。でもこれでは文字数が10文字だったのか、それ以上だったのか判別できません。知りたいのは最大文字数が未知の場合です。

こう使えば安全という使い方があればぜひご紹介ください。

回答No.1

jacta · ベストアンサー

こんなのはどうでしょうか？

if (scanf("%10s%c", buf, &c) < 2 || isspace(c))
　puts("10文字以下だった");
else
　puts("10文字を超えてちょん切られた。");

あまりパッとしませんね。

お礼

ありがとうございます。

なかなかよい案だと思います。ただ、10文字を超えていたとき次の読み込みで、文字列を繋ぎ修復する処理が厄介ですね。

回答No.12

jacta

> scanf()系の関数は、大変便利なのでつい使いたくなりますが、どういう仕様だったらもっと安全だったのでしょうねえ。

効率や利便性を犠牲にすれば、より安全にすることはできますが、外部I/Oを扱う関数を、単体で完全に安全にすることは不可能です。

Cの精神では、関数を安全に使うのは利用者の責任ですから、運用面も含めて安全になるように設計することが不可欠です。そういう意味では、#7のrinkunさんの指摘が一番的を射ているかと思います。

回答No.11

jacta

> mktemp(3)を使うのに対し、
> sprintf(dbname, "/tmp/randword%x", (int) getppid());
> とかくのは本質を外れますが、過剰品質とはいえないと思います。

もし質問の内容が「mktempの使い方を教えてください」であれば、過剰品質ではなく、単なる見当違いです。

また、一時ファイル名の生成方法に関する質問であれば、mktempに比べてご提案の方法は決して安全性が高いわけではありませんし、移植性も低下します。

さらに、質問者が環境非依存のプログラムを書いている場合や、質問中で環境を指定していない場合には、tmpnam以外の選択肢がないことも事実です。そんな場合に「環境非依存」という要件を無視して、特定環境における安全性を主張しても無意味です。

> > 作成依頼をしたいのであれば、相応の対価を支払った上で委託するのが筋です。
> 教えて！gooに出てくる質問だって、その多くは本来、対価を支払って学校などで教わるものではないでしょうか？

学校で教わることができるというだけで、ここで質問していけないわけではありません。それに対して、作成依頼目的の利用は利用規約で禁止されているので、ここでは行うべきではないのです。

回答No.10

jacta

せっかくですので、もう少し考察してみましょう。

scanf("%10ls", wcs);

のような場合には、どこまで読み込まれたのかを知るのは結構大変です。
特に、不正列によって走査が中断されると、検出するのはかなり難しくなります。少なくとも、MB_CUR_MAXバイトまでは先読みしないといけないわけですが、ungetcで戻せるのは1文字までしか保証されないのも厄介です。
やってやれないことはないでしょうが、かなりのボリュームになってしまいそうです。

scanf系の関数でどうしようもないのは、バッファオーバーフローよりむしろ数値のオーバーフローです。
入力された値が、実引数で指定したオブジェクトで表現できなかった場合の動作は未定義なので、数値がオーバーフローした時点で、一見動いているように見えても、安全性・信頼性の観点からは既に終わってしまっています。

というわけで、上記のような場合は、scanf系関数はsscanfも含めて、安全とはいえないでしょうね。

お礼

ありがとうございます。

sscanf(3)にも危険性はあるという指摘、大変興味深いです。
scanf()系の関数は、大変便利なのでつい使いたくなりますが、どういう仕様だったらもっと安全だったのでしょうねえ。

回答No.9

jacta

> 実例として、
> http://oshiete1.goo.ne.jp/kotaeru.php3?q=1719642
> からあえて取らせていただきました。文脈から悪い例と判断されるので、弊害はないかと思います。

はい。そこでの回答でも、移植性がないこと、どうすれば移植性を持たせられるかについても書いています。

> > if (scanf("%10[" CHARSET "]%c", buf, &c) < 2 || strchr(CHARSET, c) == NULL)
> この文は期待したようにはコンパイルされないと思います。

コンパイル＆動作を確認済みですが、どの辺りが気になりましたか？
まあ、動作に関していえば、scanfが0やEOFを返した場合の対策を施していないので、安全性に問題があるといえばいえなくもありませんが...

> 安全性は過剰品質ですか？

説明のためのサンプルコードでは、一般的な安全対策も過剰品質でしかありません。説明の本質から外れたコードが増えると、要点がぼやけてしまいます。

たとえ、サンプルコードを丸写しして問題が発生しようと、それはその人の問題に過ぎません。ここはソースコードの作成依頼目的で利用すべき場所ではないのです。作成依頼をしたいのであれば、相応の対価を支払った上で委託するのが筋です。

お礼

たびたびありがとうございます。

> コンパイル＆動作を確認済みですが、どの辺りが気になりましたか？
勘違いです。失礼しました。

> 説明のためのサンプルコードでは、一般的な安全対策も過剰品質でしかありません。
mktemp(3)を使うのに対し、
sprintf(dbname, "/tmp/randword%x", (int) getppid());
とかくのは本質を外れますが、過剰品質とはいえないと思います。

> 作成依頼をしたいのであれば、相応の対価を支払った上で委託するのが筋です。
教えて！gooに出てくる質問だって、その多くは本来、対価を支払って学校などで教わるものではないでしょうか？

回答No.8

rentahero

さらに蛇足かもしれませんが…

> (コード略)
> この文は期待したようにはコンパイルされないと思います。

え？正しくコンパイルされますよ。プリプロセッサの動作と、文字列リテラルの連結についてよく考えてみましょうよ。

--サンプルプログラムここから
#include <stdio.h>
#include <string.h>
#define CHARSET "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
int main()
{
char buf[16], c;
if (scanf("%10[" CHARSET "]%c", buf, &c) < 2 || strchr(CHARSET, c) == NULL)
{
puts("10文字以下だった");
}
else
{
ungetc(c, stdin);
puts("10文字を超えてちょん切られた。");
}
return 0;
}
--サンプルプログラムここまで

これでちゃんと動作します。"%10[A-Za-z]%c"と同等です。

> 安全性は過剰品質ですか？

商品なら安全性は品質です。
サンプルコードの場合は、本題以外の部分は安全性より簡単であることが重視されます。

でも、初心者向けのサンプルプログラムで、上記のようにやっちゃうと、当の初心者はついてこれません。だから、scanf("%10s", buf);で十分なのではないかと思うんです。ちなみに、私が文字列を取り出すサンプルを書く場合は、

--サンプルコード片ここから
char buf[16];

fgets(buf, 10, stdin);
buf[10]='\0';
--サンプルコード片ここまで

とかしますけどね。(これで十分なのかはまた別の問題）

ところで、これでわかるように、元の質問
> こう使えば安全という使い方があればぜひご紹介ください。
への回答は#5のjactaさんの回答で既になされているわけなんですけど、このまま続けます？

お礼

>> この文は期待したようにはコンパイルされないと思います。
失礼いたしました。勘違いです。

> 回答は#5のjactaさんの回答で既になされているわけなんですけど、このまま続けます？
「教えて！goo」の仕様で締め切ってしまうと、まだ続けたい人がいたり、回答が間違っていて補足をしたくてもできなくなってしまうのですね。
あまり早い締め切りも、いつまでも締め切らないでいるのもどっちも問題です。

しばらく新しい投稿がなくなったのを確認してから締め切らせてください。

回答No.7

rinkun

> 安全なライブラリの使い方というより、危険なプログラムを安全に使う限定方法ですね。

危険のない環境では危険なプログラムではありませんよ。
仕様外の入力を考慮しなくて良いプログラムではfscanf()もscanf()も安全に使えます。

> お言葉ではございますが、質問した初心者は、安全性など考えずにいただいたコードをありがたく使いますよ。

たいていはそれで問題も起きないので良いのでは?
もしセキュリティを考慮しなければいけないようなプログラムでそんなことをするバカだったら早めに痛い目に遭って学習させる方が身のためだし。
# そんなバカにセキュリティを考慮すべきプログラムを書かせるバカも

お礼

ありがとうございます。

バカを利口に教育してこそ「良回答」。
まあその場合は「過剰品質」かもしれません。

回答No.6

rentahero

他人の回答についた補足ですが…

> 質問した初心者は、安全性など考えずにいただいたコードを
> ありがたく使いますよ

なるほど、そういうことね、質問の意図がやっとわかりました。

初心者は確かにそのとおりだと思います。
でも、私たち職業プログラマはそうではいけないので、お客様に納品するべきプログラムではそのようなコードを使うことはありません。

というか、初心者は実際に痛い目にあわないと痛い目について理解できないので、サンプルプログラムが危険でもいいという考え方もあるのです。

--サンプルプログラムここから
int sub(char *x)
{
return scanf("%s", x);
}

int main()
{
char string[16];
return printf("%s\n",sub(string));
}
--サンプルプログラムここまで

というプログラムで、メモリを超えて入力したらおかしくなった、という経験をしないとscanfが怖いとは思わないということです。ちなみに、この例はもろにスタックを破壊しておかしくなります。
その時点で、fgetsとsscanfで代替できるということを覚えても遅くはないのでは？ということです。

お礼

ありがとうございます。

失敗に学べということどすね。
でも、IE,firefoxともBuffer overflowによるバグで、パッチを当てまくっているという現実があります。

回答No.5

jacta

> fscanf(stream, "%[A-Za-z]", str);
> 見たいな場合はどうしましょうかね？

まず、"%[A-Za-z]"という書き方には移植性がありません。これを見た初心者が真似をすると危険という観点からは、好ましくありませんね。

今回の場合も、同じようにやればそこそこの安全性は保たれるように思います。

#define CHARSET "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

if (scanf("%10[" CHARSET "]%c", buf, &c) < 2 || strchr(CHARSET, c) == NULL)
{
　puts("10文字以下だった");
}
else
{
　ungetc(c, stdin);
　puts("10文字を超えてちょん切られた。");
}

今回の場合に特化すれば、isalphaでも構いませんが、一般的にはstrchrまたはmemchrにする必要があるかと思います。

ところで、個人的には

> 説明のためのサンプルコードにセキュリティ上の安全性を求めるほうが間違ってる。

を指示します。
過剰品質は、時としてセキュリティホール以上に問題がありますから。

お礼

たびたびのご回答ありがとうございます。

> まず、"%[A-Za-z]"という書き方には移植性がありません。
実例として、
http://oshiete1.goo.ne.jp/kotaeru.php3?q=1719642
からあえて取らせていただきました。文脈から悪い例と判断されるので、弊害はないかと思います。

> if (scanf("%10[" CHARSET "]%c", buf, &c) < 2 || strchr(CHARSET, c) == NULL)
この文は期待したようにはコンパイルされないと思います。

> 過剰品質は、時としてセキュリティホール以上に問題がありますから。
過剰品質は要求していません。安全性は過剰品質ですか？

回答No.4

rinkun

安全な使い方といえば、入力にオーバーフローするようなデータがないと分かっているときや、オーバーフローを起こしても対処できるような使い方しかしないとき。
あらかじめフォーマットが確認されているファイルをfscanfで読み込むとか、作った本人が使うプログラムでscanf入力を使うとか。

> このカテゴリの回答の中にも多数のscanf()を使った回答を見かけます。

説明のためのサンプルコードにセキュリティ上の安全性を求めるほうが間違ってる。
# 話題自体がセキュリティの場合は別だが

お礼

ありがとうございます。

安全なライブラリの使い方というより、危険なプログラムを安全に使う限定方法ですね。

> 説明のためのサンプルコードにセキュリティ上の安全性を求めるほうが間違ってる。

お言葉ではございますが、質問した初心者は、安全性など考えずにいただいたコードをありがたく使いますよ。サンプルコードにも安全性は求められると思います。汎用性までは求めなくとも。

回答No.3

jacta

#1です。

> ただ、10文字を超えていたとき次の読み込みで、文字列を繋ぎ修復する処理が厄介ですね。

10文字を超えていた場合に読み込んだ次の文字は、ungetcで戻してやれば少し楽になりそうです。

お礼

ありがとうございます。

良い案ですね。でも、"%s"でなく、
fscanf(stream, "%[A-Za-z]", str);
見たいな場合はどうしましょうかね？