strcpyも使用しない

107日前原文(daniel.haxx.se)

概要

curlプロジェクトでは、**strncpy()とstrcpy()**の使用を廃止し、安全な文字列コピー関数へ移行。
**strncpy()**はAPIが不適切で、誤用リスクが高い関数。
**strcpy()**も安全性の観点から独自関数に置換。
新関数はバッファサイズや文字列長を明示的に扱い、バグを未然に防止。
AIによる誤検出も抑制可能だが、根本的な解決には至らない現状。

curlにおけるstrncpy()廃止の経緯

**strncpy()**はAPI仕様が分かりにくく、誤用によるバグ発生源
バッファ終端のnull文字未保証、余分なゼロ埋め発生
多くのコードベースで**strncpy()**の利用自体を避けるべきとの判断
curlソースコードから**strncpy()**の呼び出しを完全廃止
文字列全体のコピーができない場合はエラーを返す方針
部分コピーが必要なケースはmemcpy()＋明示的なnull終端で対応
strlcpy等の代替関数も不要と判断

strcpy()の課題と置換理由

**strcpy()**には一定の有用性があるが、APIが不十分
バッファサイズや文字列長の明示的指定ができない
利用時は事前にバッファサイズチェックが必要
長期運用・複数人開発でチェックと呼び出しが分離しやすく、リスク増大
バッファサイズチェックとコピー処理を密接に結合する必要性

独自string copy関数の導入

curlx_strcopy関数の新設
引数：ターゲットバッファ、バッファサイズ、ソースバッファ、ソース文字列長
コピー可能かつnull終端が収まる場合のみコピー実行
実装例：memcpy()＋明示的なnull終端処理
**strcpy()**の完全禁止が可能に

利用は若干手間だが、安全性・保守性を優先

void curlx_strcopy(char *dest, size_t dsize, const char *src, size_t slen){
  DEBUGASSERT(slen < dsize);
  if(slen < dsize) {
    memcpy(dest, src, slen);
    dest[slen] = 0;
  }
  else if(dsize)
    dest[0] = 0;
}

AIによる脆弱性誤検出の抑制

**strcpy()**のコード残存はAIによる脆弱性指摘の温床
独自関数への統一でAIの誤検出を抑制
ただし、AIは他の箇所でも誤検出を継続する可能性
AIによる誤検出問題の根本解決には至らない現状

Hackerたちの意見

記事からの引用: 「ソースコードのstrcpyは、幻の脆弱性主張を生み出すためのハニーポットのようなものだということが、何度も証明されている。」この記事のこの締めくくりがすごく印象に残った。AIがCコードをチェックする意味って何なんだろう？strcpy()を問題として指摘するだけなら、何の注意もなしに。

└

人間ってバカだから、AIが得意じゃないことに使うんだよね。

└

記事が示唆しているほど単純じゃないよ。幻の脆弱性レポートは「注意なし」で指摘するわけじゃなくて、どこかに論理的な誤りがある複雑な脆弱性の証明を作り上げて、それが脆弱性レポートとして提出されるんだ。だから、メンテナンスする側にとってはイライラするんだよね。「証明」を読んで矛盾を見つけなきゃいけないから。

└

OSSコードにAIチェックをかけて、偽のバグレポートを提出する人たちは、AIが間違えないと思っているか、報告が正当かどうか気にしないんだよね。だって、たとえそれが正当じゃなくても、彼らにはほとんど個人的なコストがないから。

curlx_strcopyが成功を返さないのには驚いた。確かに、dest[0]が'/0'じゃないかをチェックすることもできるけど、それは書くのが面倒だし、ミスも起きやすいから、成功をチェックするのは推奨されてないんだよね。

└

もしこの行でコードがクラッシュしなければ、DEBUGASSERT(slen)が成功を意味するっていう考えなんだろうね。ただ、いくつかのコンパイラはリリースビルドでアサーションを削除しちゃうけど。明示的なエラーコードの方が良かったな。

└

これが特に奇妙なのは、彼が上で「部分的な文字列をコピーするのが正しい選択であることは稀だ」と説明しているのに、前の解決策がエラーを返したことだよね… それで今は、何も部分的にコピーせずにdestを空の文字列に設定して、静かに失敗するってこと！？

└

そうだね、私も同じこと思った。今後、いくつかのCVEが出るかもね。

変なAnnex-KみたいなAPIだね。宛先バッファのサイズには末尾のヌル文字のスペースも含まれてるけど、ソースのサイズはヌル以外の文字バイトだけを含んでる。これって、strcpyの代わりにmemcpyを使わせることに何の意味があるんだろう。

Cの様々な文字列ルーチンの動機についてずっと疑問に思ってたんだけど、どれも大きな注意点があって使えないんだよね。何年も経って、文字列にどれだけメモリが割り当てられているかをポインタと一緒に記録するライブラリが必要だと思うようになった。こんなのがあればいいな: https://github.com/msteinert/bstring

└

strncpyは結構簡単で、C文字列を固定幅の文字列にコピーするための特別な関数なんだ。昔のCアプリケーションのディスクフォーマットでよく使われてた。例えば、最大20文字を含むchar username[20]フィールドがあって、使われていない文字はヌルで埋められる。これがstrncpyの目的。宛先引数は常に固定サイズのchar配列であるべきだね。数年前にAlejandro Colomarのおかげで、これに関する新しいマニュアルページができたよ: https://man.archlinux.org/man/string_copying.7.en

└

これはコンピュータウイルスが登場する前の時代の話だよね？でも、こういう帳簿管理は余分な時間とスペースを取るから、今の時代では簡単にトレードオフできるよね。

└

それでも、Cで開発されたソフトウェアは、文字列ルーチンの欠点を抱えながらも、何年も売られていて、総売上は数兆ドルに達している。文字列にどれだけメモリが割り当てられているかを記録するライブラリは必須ではないんだ。Cでプロとして書いている人たちは、これに完全に慣れているけど、足元をすくわれることは（他の問題も含めて）常に潜んでいる。一般的には、こんなコードを見ることが多い:- char hostname[20]; ... strncpy(hostname, input, 20); hostname[19]=0; 問題は、最後のバイトをNULにする行を忘れて、かつ入力が19文字を超えるときに発生する。これを間違えるのはとても簡単で、最初は`hostname[20]=0;`って書きそうになったこともある。20年以上前、顧客のサイトでSybase Open/Serverを使ったソフトウェアが起動時にクラッシュする問題をデバッグしたことを覚えている。基盤となるTDS通信プロトコル（https://www.freetds.org/tds.html）にはホスト名用の固定30バイトフィールドがあって、顧客は特に長いFQDNをチェックなしでコピーしていた。特定されれば簡単に修正できる問題だったけど、当時はバッファオーバーランの結果は、ランダムなクラッシュやモリスワームのような軽い迷惑で済むことが多かった。今では、そのようなバッファオーバーランは非常に深刻で、データの流出やRCE、完全な侵害につながる可能性がある。HeartbleedやMongobleedはCの文字列関数とは関係なかった。どちらも、ユーザーが提供したペイロードの長さを信頼したことが原因だった。（Cの文字列関数は、今でも問題の大きな原因だけどね。）

└

そうだね、文字列に長さがないのは間違いだったよ。昔はバイトが貴重だったから、長さに1バイトじゃなくて2バイト使うのは大きな損失だと思われてたんだ。最初に「varint」みたいなのがあったらどうなってたんだろうってずっと考えてたけど、その時代のことはよく分からないから、感覚が掴めないんだよね。

└

>「Cのさまざまな文字列ルーチンの動機についてずっと不思議に思ってた」このイディオム: char hostname[20]; ... strncpy(hostname, input, 20); hostname[19]=0; は、strncpyが14バイトの配列に保存されるファイル名をコピーするために発明されたから存在するんだ。スペースが許せばヌル終端になる（https://stackoverflow.com/a/1454071）。

└

>「どれも大きな注意点があって、役に立たないように見える」それらはCに追加されたとき、設計していた人たちがその影響を十分に理解していなかったんだ。もう一つ根本的に壊れた見落としは、関数のシグネチャでの配列からポインタへの降格で、太いポインタ型を持たないことだね。

> サイズチェックがコピー自体から分離できないようにするために、先日、ターゲットバッファ、ターゲットサイズ、ソースバッファ、ソース文字列の長さを引数に取る文字列コピーの置き換え関数を導入したんだ。コピーができて、ヌル終端もそこに収まる場合だけ、操作が行われる。… もしコピーができなかったら、どうやら宛先はスペースがある限り切り詰められるみたい（つまり、要素0にヌル終端が書き込まれる）。そして、戻り値はvoid。コピーが不可能な場合の対処法としてこれがベストだとは思えないな。エラーケースは非ゼロの戻り値で通知すべきだと思うし、宛先バッファはそのままにしておくべきだよね。確かに、そういうことは起こるべきじゃない（だからDEBUGASSERTマクロがあるんだろうけど）、でもやっぱり。最初にチェックするのを呼び出し元の責任にするより、その可能性を考慮して設計した方が簡単かもしれない。

> コードに近いところでチェックを強制するこれはすごく理にかなってるけど、データセットのライフタイムの早い段階でチェックが必要な時に、これがややこしくなることがあるんだ。何度もチェックするためにコストをかけたくないけど、チェックを上に押し上げると、将来のリファクタリングで見失っちゃう。だから、基本的に「このデータに作用するには、まずチェックされている必要がある。いつチェックされたかは気にしない、今までにチェックされていればいい」と言うメタデータを想像してる。これってRustがResult型でやってることなんじゃないかな？その時点で、チェックがコードにどれだけ近いかは関係なくなるよね、チェック済みと未チェックを区別できれば。

└

それには異なる型を使うべきだね。JavaのStringとCharSequenceみたいに。

└

> 私が想像しているのは、RustがResult型でやっていることかな？Resultは、特定の操作の成功/失敗に関する情報しか持っていなくて、長期的なシグナルではないし、さらに改ざんにも耐えられない（だからResultの処理ミスが検証を台無しにすることがある）。この場合、チェック操作を通じてしか構築できない新しい別の型が必要だよ。「検証するな、パースせよ」という考え方だね。君の言う通り、その場合、チェックは消費者の近くにある必要はなくて、むしろ逆で、チェックはソフトウェアのエッジにできるだけ近くにあって、汚染されたデータがシステム内にほとんど存在しないか、全く存在しないようにするべきなんだ。そうすれば、無意識にそれとやり取りするのが難しくなるからね。でももちろん、その方向に進むほど、より表現力のある型システムが必要になる。いくつかの制約は簡単にはチェックできないし、各消費者がそれぞれの癖を持っているから、あるいはこの場合のように複数のランタイムオブジェクトの相互作用をチェックする必要があるからね。

注意すべきは、strcpy()はセキュリティの観点からだけでなく、古くないCPUではパフォーマンスの観点からも良くないってこと。最良のケースを考えてみて、正確な長さは不明だけど、例えば64バイトに収まることは分かっている文字列をコピーする場合。昔は、このタスクには必ずstrcpy()を使って、「無駄な」余分なコピーを避けていたよね。効率的に感じたし、結局はループ内でi < lenのチェックをbuf[i] != nullに置き換えるだけだったからね。でも、実際にはそううまくはいかなくて、1バイトずつコピーするのは非効率的だから、できるだけ一度にコピーするようにするんだ。これは長さチェックだけで簡単にできるけど、ヌルバイトを見つける必要があるときはそう簡単じゃない。そして、その上でCPUに入力データに完全に依存する分岐を予測させることになる。

└

できるだけ早く、ヌル終端文字列から離れるべきだよね。

└

最近、strcpyがスカラーのループを使ってるのを見たことないな。これってARMの話？

「strncpy()は変な関数で、クソみたいなAPIだ。」まあ、元々ヌル終端されていない文字列用に作られたって調べれば、なんとなく納得できるよね。本当の問題は、静的解析ツールがstrcpyの代わりに使うことを推奨し始めたときから始まったんだ（本当の代替手段はsnprintfだったけど、今はstrlcpyだね）。

└

君のコメント、意味がわからないよ。もしヌル終端文字列用に設計されてたなら、なんでヌル終端の後にパディングする必要があるの？実際の理由を調べてみたんだけど、ANSI Cプログラミング言語の理由書によると、strncpy関数は、ディレクトリエントリのような固定長の名前フィールドを扱うためにCライブラリに導入されたんだ。こういうフィールドは文字列とは使い方が違って、最大長のフィールドには末尾のヌルは必要ないし、短い名前のために末尾のバイトをヌルにすることで、効率的なフィールドごとの比較ができるんだ。strncpyは元々「制限付きstrcpy」ではなくて、委員会はその機能をより適した使い方に変更するよりも、既存の慣習を認めることを優先したんだ。

└

strlcpyはBSDのもので、posixにはないんだ。公式の推奨はstpcpyなんだけど、残念ながらドキュメントにしか実装されてなくて、どこにもないから自分で作らないといけないんだ: https://man7.org/linux/man-pages/man7/string_copying.7.html。

バイクシェディングだね。

引数のオフバイワンのバッファサイズと文字列の長さの問題、すごく使いにくいと思うし、今後さらに使用ミスを引き起こすだろうね。バイクシェディングの学位持ってるけど、なんでいつもこの質問されるんだろう。