strcpyは常に危険か?

当然ですが、Noです。安全に使うことが非常に難しいだけです。

strcpyの定義は

char *strcpy(char *dst, const char *src);

ですが、以下の条件が全て成立しなくてはならない事に問題があるのです。

  • 書き込み先(dst)のサイズを知っている
  • 読み込み元(src)が「”」で終了している
  • 読み込み元文字列の長さが書き込み先のサイズを超えない


C言語には安全に書き込めるサイズを後から知る方法が存在しません。つまり、確保したときに記憶し、それが変更されない事を保証する必要があります。これはスタックであってもヒープ(フリーストア)であっても同様です。保証出来ない場合、バッファオーバーフローの脆弱性が存在します。

次に、引数でサイズを渡すstrncpyを利用したら良いサイズを渡したらと考えたとしましょう。実は、これも(実装者や攻撃者が)正しい長さを渡す保証が何処にもないため危険な場合が存在するのです。

そこで、strncpyに似た動作を含むコードをより安全に実装してみましょう。関数自体が取り得るコピー先の最大長を知っている場合において、利用できるテクニックの1つを紹介します。





void func(char *dst, size_t dst_len, const char *src){

  char szFileName[MAX_PATH+1];
  dst[dst_len] = 0xCC; // if dst_len is invalid-param, it will be failed.(AccessViolation)

  if (dst_len < MAX_PATH)
  {
    strcpy(dst, src); // safely
  }
  // ・・・
}

この例では、新たな課題である不正なサイズが(間違ってor意図的に)渡される事を防止しています。

謎の「0xCC」代入がそれで、不正な長さが与えられた場合、アクセス違反で落ちる事を期待した実装です(0xCCはx86でint3命令であり、仮に読み込まれたとしてもbreakになります)。私の例はオーパフローを利用した攻撃を食らう前にアプリを終了させます。

そんな馬鹿な方法・・・とお思いの方はMicrosoftが実装した「安全なstrcpy関数」である以下の2つの関数を参照してみてください。

  • strcpy_s // <cstring> visual studio 2005 以上
  • StringCchCopy // <strsafe.h> IE 5.5 以上

私のコードとの違いは、AccessViolationをSEH(WindowsOSの例外)で捕捉し、関数を失敗させるのに留める点です。

純粋なC++で片付けないのがMicrosoftらしいと言えますが、仮にバッファオーバーフローの問題が存在しても、アプリを落とさず、且つ安全に動かせるわけです。

再度いいますが、私どもがstrcpyを安全に使う方法は難しいのです。

文字列に関しては以下のガイドラインを遵守してください。

  • 固定長バッファを使わず、安全な「stringクラス」を利用しよう
  • 固定長バッファを使うときでもstrcpyを使うことを辞めよう

どうやっても安全に使えない関数もあります。例を挙げるとscanfで、これは絶対に利用しないでください。

なお、ネタ元になった「Writing Secure Code第2版」には、もう少し高度なセキュリティ問題が書かれています。興味のある方はご一読を。



(2006/9/6) 誤解を招く表現を訂正。まだ更正中。



[技術書レビュー] – Writing Secure Code第2版 上・下

Leave a Reply

6 コメント - "strcpyは常に危険か?"

更新通知を受け取る »
avatar
並び替え:   新しい順 | 古い順 | 最も評価の多い
boo
ゲスト

過去の記事に突っ込み入れるのもどうかと思いましたが、根本的な部分から間違っているので指摘させて頂きます。
まず一つ目。
dst[dst_len] = 0xCC; // if dst_len is invalid-param, it will be failed.(AccessViolation)
ここで必ずオーバーランが発生しています。記述するならdst[dst_len – 1] = 0xCC; です。
仮にdst[dst_len – 1] = 0xCC;としても攻撃者のコードを防ぐことはできません。極端な例ですが、
void overrun()
{
char a[1];
func(a, MAX_PATH – 1, 悪意のあるコード);
}
とした場合、0xCC が書き込まれるのは MAX_PATH – 1 – 1 です。func()をコールした後のスタックは
 000:a[0]
 004:呼び出し元のアドレス ←破壊されて008に。
 008:悪意のあるコード
  :
  :
  :悪意のあるコード
 MAX_PATH-1-1:0xCC
という感じになります。0xCC を書き込んでいる意味が無いことがお分かりかと思います。
二つ目。
strcpy_s()がセキュアなのは必ずバッファ長を渡すことに起因するのであって、AccessViolation
のチェックは関係ありません。AccessViolationの部分はNULLチェックを発展させたというだけの
ものです。IsBadReadPtr/IsBadWritePtr などを調べてみてください。
スタックの関数の呼び出し元が書かれている部分に書き込んでもAccessViolationは
おきませんし、スタックを破壊しまくった後に関数が失敗しましたと戻っても意味がありませんよね?
(その戻る先は書き換えられていて戻った先は攻撃者が用意したコードかもしれません。)
三つ目。
つまり、バッファのサイズが分からなくなった時点で手遅れなんです。ご自身で
「保証出来ない場合、バッファオーバーフローの脆弱性が存在します。」と書いていらっしゃるじゃないですか。
「不正なサイズが(間違ってor意図的に)渡される事を防止」することはその時点で不可能なんです。
バッファーオーバーフローを防ぐにはバッファのサイズを常に保証できるようにしそのサイズを超えた
アクセスは行わないこと、しかありません。バッファのサイズを保証できなくてもチェック次第で
攻撃者のコードは防げるという考えは捨てて下さいませ。

konuma
ゲスト

コメント、ご指摘ありがとうございます。
>ここで必ずオーバーランが発生しています。
それは「プロービング」です。
5文字の文字列を書き込むにはを含めて6文字が必要です。その6文字目へのアクセスが成功するかのテストです。つまり、正常にメモリに書き込めるかのテストであり、攻撃を完全に防ぐコードではありません。
>バッファのサイズを保証できなくてもチェック次第で攻撃者のコードは防げるという考えは捨てて下さいませ。
当然、そんな事はおもっていません。
指摘を元に記事を書き直そうとおもいます。
有り難うございました。

水無瀬
ゲスト

AccessViolation によるチェックがある場合と無い場合の違いについてバッサリと書いてみる。
以下のような関数呼出(直に定数が書かれることは実際にはないだろうが要点は同じ)について考える。
 char buf[ 5 ];       // サイズ 5 の領域に、
 size_t len = 10;      // 10 文字(11バイト)まで書き込んで良い!?
 const char *str = "a"; // が書き込む文字列は 1 文字(2バイト)しかない。
 func( buf, len, str );
このコードはサイズ 5 の領域に 10 文字の書き込みを許可しており誤りであるが、
書き込む文字列の長さが 1 である(サイズ 5 の領域中の 2 バイトしか利用しない)ため問題が顕在化しない可能性がある。
しかし AccessViolation によるチェックがある場合は、
許可された長さの限界位置に実際に書き込みが行われる (dst[dst_len] = 0xCC;) ため
問題が発見される可能性が(AccessViolation によるチェックが無い場合よりは)期待できる。
……という理由で私も dst[dst_len] = 0xCC; を長年愛用しています。

にかわ論者
ゲスト

ここでちょっと実験。
VS2005 で、下記のプログラムを「WIN32コンソールアプリ」「Debug 構成」
でビルドして実行すると、"Hello World!" *表示後に*
「Run-Time Check Failure #2 – Stack around the variable 'a' was corrupted.」
になります。
ところが、(*) の 0xcd を、0xcc に変えると一見正常に動いてしまいます。
#include
int main(void)
{
char a[1];
a[1] = 0xcd; // (*)
printf("Hello World!n");
return 0;
}
この結果から、デバッグビルドにおいて、VC は、配列の直後に、内容が 0xcc の PAD 領域を挿入し、それがプログラム実行後に
書き換えられたか否かで Access Violation を判定する、というアルゴリズムを採っているのだと思われます。
だとすると、
・デバッグビルドでの明示的 dst[dst_len] = 0xCC; は単なるムダ
・リリースビルドでの明示的 dst[dst_len] = 0xCC; は、多くの場合隣の変数を壊すだけ(顕在化しないバグになる)
ので全く感心できません。
なお、ちょっと不勉強なんですが、Access Violation が起きたことを例外で知らせてくるのって、
ヒープのリンクの破壊が(ソフトウェア的に)検出された等、ランタイムライブラリ内で assertion に引っかかったときと、
該当プロセスに割り当てられていないページへのアクセスが(ハードウェア的に)検出された場合だけではないでしょうか。
スタック上の一配列のオーバーランではいちいち例外は上がらないと思うのですが。

にかわ論者
ゲスト

> 例外で知らせてくるのって(中略)該当プロセスに割り当てられていないページへのアクセスが(ハードウェア的に)検出された場合だけではないでしょうか。
すいません寝ぼけてました。他にも 0 による除算とか未定義命令とか、他にもいろいろありますたね orz
ですがとにかく、スタック上の一配列のオーバーランを即座に検出する仕組みはさすがにないと思います。

konuma
ゲスト

コメント有り難うございます。
booさんを初めとして、色々と指摘が入ったのは私が問題の切り分け方を間違ってしまったからだと考えています。
そもそも、文字列のコピーだけでも
・必須の確認
・推奨の確認
・デバックビルドで行うべき確認
で分けるべきだったのです。
そのうち綺麗に纏めていきたいと思いますが、最近忙しいためお待ちください。
>・デバッグビルドでの~
とも限りません。デバックの場合、偶然に書き込めてしまう場合がありますが、問題を発見できる可能性があります。
>・リリースビルドでの明示的 ~
呼び出し元が不正ならば、単に壊すだけです。
デバック段階で除去してしまう方が一般的と思われますが、Release版でテストしないと問題が出ない場合があるため、ギリギリまで残すべき、状況によっては残したままで良いと考えてます。
>Access Violation が起きたことを例外で知らせてくる
WindowsOSは全てのヒープ領域をカーネルオブジェクトとしてテーブルにもっています。その範囲外の時に例外を投げますが、同じプロセスの別のヒープに偶然書き込めてしまった場合、例外が投げられないのかもしれません。
スタックについてはVC++でいう/GSスイッチあたりがカナリア値を埋めるため、1バイト程度のオーバーライトは高確率で検出できます。
>スタック上の一配列のオーバーランを即座に検出する仕組み~
その通りです、有りません。
いかんせん、私のかいた文章に誤解を与える表現が有ることは分かっているため、そのうち書き直します。
ありがとうございました。

wpDiscuz