かなり古いC++(VC6.0)で書かれたプログラムのメンテナンス依頼があったので久しぶりに C++ をごにょごにょいじってました
まずは、下記ソース見て下さい
わかりやすく少し省略して、コメントを追加してあります
数値を指定の長さで書式化する関数で、option には "9" とか "X" とかの1文字の書式文字が入ってきます
void CLASS1::hogehoge(void *dest, long number, int size, const char *option) { char *result; char buf[10]; result = new char[size + 1]; memset(result,0x0,(size+1)); if (option == "9") { // 罠1 sprintf(buf,"%%%dd",size); sprintf(result,buf,number); } else { sprintf(buf,"%%%s%dd",option,size); sprintf(result,buf,number); // 爆弾投下!! } memcpy(dest,result,size); delete [] result; // 爆発!! }
これは sprintf が最悪なバッファオーバーフローを引き起こすパタンで、2重に罠がしかけれれてます(笑)
みつかったのは、VC2005のデバッガーモードのメモリアロケーション処理が異常を指摘してくれたからなのですが、この手のバグは結果が現れたのが原因を引き起こした場所ではなく、全然関係ない場所でクラッシュするので厄介です
この場合は delete [] result でメモリ解放時に、下記のようなエラーがでました
---
HEAP[fugafuga.exe]: Heap block at 089F8300 modified at 089F832F past requested size of 27
Windows によって fugafuga.exe でブレークポイントが発生しました。
ヒープが壊れていることが原因として考えられます。fugafuga.exe または読み込まれた DLL にバグがあります。
---
number = 1 、 siza = 2 、 option = "9" として、机上でシュミレーションしてみます
この関数を作った人は、以下のような動きを想定してたのだと思います
if (option == "9") になるから、 書式は(buf値)「%2d」となる
これで結果を書式化して、result は 「△1」(△=空白)になる
まぁ実際 VC6.0 でトレースすると上記のような動きになります
でも、VC2005 ではそうはなりません
そもそも、この関数には重大なミスがあるんです
罠1です
option は文字のアドレスなので「if (option == "9") 」は、アドレス値とリテラル文字列を比較しています
この比較ではまず等価になることはありえません
なので、書式を決める部分は else が処理され、結果の書式は(buf値)「%92d」となります
つまり %s の部分が option 文字の 9 になっちゃうんですね
これで sprintf して爆弾投下です(笑)
つまり、result には、2文字分のエリアしか確保していないのに、92文字もの文字列がコピーされて、バッファオーバーフローを引き起こすわけです
そして何処かで爆発するわけです
ただ、かわらないのが、何故このロジックが VC6.0 で正常に動いていたかです
実際にトレースしてみても正常に動きます
なんでだろ??
コンパイラが世話を焼いてくれてるのか、「==」がどこかでオーバーロードされてるのか??
まぁ時間もないので、これ以上は調べませんでしたが
VC6.0 でも VC2005 でも、コンパイルの警告レベルを最大にすると、上記のバグは警告してくれます
---
C4130: '==' : 文字列定数のアドレスで論理演算が行われました。
---
オイラが開発するとき、プロジェクト構成の設定でまずやるのが、この警告レベルを最大にすることです
小姑みたいに細かいこと言われますが、けっこうケアレスミスを防げます
ちなみに、文字列の比較は「strcmp」を使って
「if (0 == strcmp(option,"9")」が正解
Comments