0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

printf(user) の書式文字列脆弱性をcreviewで検出した記録

0
Posted at

C言語23年、自分はフリーランスで組み込みとサーバサイドの両方のコードレビューに入ることが多いです。今回書くのは printf にユーザー入力をそのまま渡すバグ、CWE-134の書式文字列脆弱性です。これ、レビューで指摘しても「動くから直さなくていいですか」と聞かれたことが実際にあります。動くんですよ、%s%n を含まない普通の文字列なら。でも攻撃者が %n を混ぜた入力を送ってきた瞬間、任意メモリ書き込みに変わる。地雷が地雷だと分かるのは踏んだ後、というやつです。

この記事の前提(SCQA)

  • 状況: printf系関数の第一引数(書式文字列)に外部入力の変数を直接渡すコードは、レガシーコードにもモダンなコードにも普通に紛れ込みます。とくにログ出力関数のラッパーで頻出します。
  • 問題: printf(user) は構文的に完全に正しく、コンパイラの -Wall では黙って通ることがあります。実行時に %s%n を含む入力が来るまで誰も気づきません。
  • 問い: コンパイラ警告で拾えない「意味的に危険なパターン」を、人間の目視レビューに頼らず機械的に洗い出すにはどうするか?
  • 答え: creview の --preset security で書式文字列パターンを専用ルールとして検出し、CWE-134として一括で報告させる運用に切り替えました。

L0: まずコンパイラ警告を全部潰す

creview の話に入る前に、-Wall -Wextra -Wpedantic -Werror -fanalyzer(GCC)または -Wall -Wextra -Werror -Weverything(Clang)を有効にしてビルドが通るか確認してください。

実はこの記事の検証コードは -Wformat-security を有効にすると printf(user) の時点でGCCが warning: format not a string literal and no format arguments を出します。つまりL0の時点で半分は拾えます。ただし今回のケースには罠があって、-Wformat-security-Wformat=2-Wall に必ずしも含まれない環境があり、CIの設定次第で素通りします。自分は過去に「-Wall -Wextra を付けているから大丈夫」と思い込んでいたプロジェクトで、この警告フラグが抜けていて本番まで到達したケースを見ています。

これらが警告ゼロにならない、あるいは警告フラグの構成自体が疑わしいPRは、creviewを走らせる前にそもそもレビュー対象にしないルールにしてしまうのが第一手です。

検証用コード

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

/* printf にユーザー入力を直接渡す(書式文字列脆弱性) */
void log_user_input(const char *user) {
    printf(user);                        /* 検出すべき: printf("%s", user) にすべき */
}

/* fprintf も同様 */
void log_to_file(FILE *fp, const char *msg) {
    fprintf(fp, msg);                    /* 検出すべき */
}

/* puts は実は安全だが、似た形の罠 */
void log_short(const char *msg) {
    puts(msg);                           /* puts は安全(書式解釈しない)が紛らわしい */
}

/* snprintf も書式引数を変数で渡すと危険 */
void format_log(char *out, size_t n, const char *user_fmt) {
    snprintf(out, n, user_fmt);          /* 検出すべき */
}

log_short の puts だけは実害がありません。puts は書式解釈をしないので %n を渡されても文字通り出力するだけです。ただしcreviewはこの関数もNULLチェック漏れの観点で拾っていて、これは後述のCWE-476として扱う話です。

CWE/MISRA マッピング

検出パターン CWE 事故の形
printf(user) 書式文字列に変数を直接渡す CWE-134 攻撃者が %n を含む入力を送ると任意アドレスへの書き込みが発生。%x の連投でスタック上の値を読み出すことも可能
fprintf(fp, msg) 同様のパターン CWE-134 ログファイル経由でも同じ攻撃ベクター。ログ関数だから安全という思い込みが危険
snprintf(out, n, user_fmt) 書式文字列に変数 CWE-134 バッファサイズ指定があっても書式解釈自体は防げない。%n は書き込み先アドレスを書式文字列側で自由に指定できる
user / fp / msg / out の NULL 未検査 CWE-476 NULLポインタを書式引数や出力先に渡すとクラッシュ、あるいは契約違反による未定義動作
snprintf 戻り値未確認 CWE-252 出力が切り詰められたかどうかを検出できず、ログの欠落や後続処理の前提崩壊につながる

puts自体はCWE-134の対象外です。書式解釈をしないので %n を渡しても文字列としてそのまま出力されるだけ。ここをcreviewが「puts の書式文字列に変数を直接使用」と表現している点は、正直に言うと自分は最初「これは誤検出では」と確認作業をしました。結論としては、puts自体は安全だが「同じ形のコードパターンが並んでいると、後で誰かがsprintf系にコピペで転用したときに気づかれにくい」という保守上のリスクを警告している、という理解に落ち着いています。

creview の使い方(CLI 実行例)

creview scan format_string_demo.c --preset security --format markdown
L6 【重大】: ATTR-NONNULL-001: log_user_input() の user を NULL 検査せず使用。__attribute__((nonnull(N))) で契約を明示するか、関数先頭で NULL 検査して return
L7 【重大】: printfの書式文字列に変数userを直接使用。攻撃者制御で任意メモリ読み書き可能
L11 【重大】: ATTR-NONNULL-001: log_to_file() の fp を NULL 検査せず使用。__attribute__((nonnull(N))) で契約を明示するか、関数先頭で NULL 検査して return
L12 【重大】: fprintfの書式文字列に変数msgを直接使用。攻撃者制御で任意メモリ読み書き可能
L16 【重大】: ATTR-NONNULL-001: log_short() の msg を NULL 検査せず使用。__attribute__((nonnull(N))) で契約を明示するか、関数先頭で NULL 検査して return
L17 【重大】: putsの書式文字列に変数msgを直接使用。攻撃者制御で任意メモリ読み書き可能
L21 【重大】: ATTR-NONNULL-001: format_log() の out を NULL 検査せず使用。__attribute__((nonnull(N))) で契約を明示するか、関数先頭で NULL 検査して return
L22 【重大】: snprintfの書式文字列に変数user_fmtを直接使用。攻撃者制御で任意メモリ読み書き可能
L22 【保守危険】: snprintf戻り値を未確認。切り詰め発生を検出できない

3段階ラベルの読み方は以下の通りです。

  • 【重大】: 実行時に外部入力次第でクラッシュや任意メモリアクセスに直結する箇所。今回のCWE-134の4件とCWE-476の4件がここに該当します。マージ前に必ず潰す前提で扱います。
  • 【設計不明】: 今回の検出には出ていませんが、意図的な設計なのかバグなのか機械では判断できないケースに付くラベルです。人間が「これは仕様です」とコメントを残すか修正するかの判断を求められます。
  • 【保守危険】: 今すぐクラッシュはしないが、将来の変更やエッジケースで問題化しうる箇所。snprintfの戻り値未確認はこれで、切り詰めが発生してもコード上は気づけません。

自分の運用では【重大】はCI red、【設計不明】はレビューコメント必須、【保守危険】はIssue化して次スプリントで潰す、という3段階の対応速度に割り振っています。

3段階の使い分け

場面 使うツール 使いどころ
L0: ビルド時(まずここ) gcc / clang の警告 + -Werror -Wall -Wextra -Wpedantic -Werror -Wformat -Wformat-security -fanalyzer(GCC)。警告ゼロにならないPRはそもそもレビューに入らないルール化
L1: PRチェック clang-tidy / cppcheck clang-analyzer-security.insecureAPI.* 系のチェックをCIで強制
L2: PRレビュー creview(CLI) プロジェクト固有パターン、日本語ナレッジ、severityモデル(重大/設計不明/保守危険)、CWEマッピング、--preset security でセキュリティ観点に絞って実行
L2': 自己チェック(PR投げる前) c-review-ai(ブラウザ版) 環境構築不要、コピペで書式文字列パターンだけ試せる軽量版
L3: チーム監査・MISRA準拠 CSAF libclang AST + 依存グラフでログ出力関数の呼び出しチェーン全体を追跡し、risk A/B/C自動昇格

L0で-Wformat-securityを付け忘れているプロジェクトは、実は自分が過去に3社ほど見ています。CMakeの共通設定ファイルをコピーして使い回している現場ほど、この手のフラグ抜けが起きやすい印象です。creviewはL0の抜け漏れをカバーする意味でも、L0とは独立したルールとして書式文字列パターンを常に走査しています。

まとめ

  • printf系の書式文字列に外部入力変数を直接渡すコードはCWE-134であり、%nを含む入力ひとつで任意メモリ書き込みに変わる
  • puts は書式解釈をしないため実害はないが、同じコードパターンの並びは将来のコピペ事故の温床になる
  • snprintfは書式文字列の危険性に加えて戻り値未確認のCWE-252も併発しやすく、切り詰め検出漏れは別枠の【保守危険】として扱う
  • -Wformat-securityはL0で拾えるはずの警告だが、CI設定のコピー元に含まれていないケースが現場で普通に起きる
  • 【重大】【設計不明】【保守危険】の3段階は対応速度の優先順位付けであり、severityが同じ「重大」でも設計判断が必要な場合は別ラベルに分けるべき

試すリンク(無料で試す → 業務で監査する)

C言語の静的解析を、いきなり有料ではなく「無料で手を動かす → 日常運用 → 監査レベル」の順で用意しています。

  • まずブラウザで試す → c-review-ai(自社プロダクト / MIT / 無料)
    貼り付けた1ファイルをその場でレビュー。導入ゼロで挙動を確かめる用。
    https://cutt.ly/pyuTypUW
  • CLI で日常運用 → creview(自社プロダクト / 無料)
    手元のコードをコミット前に単発チェック。CI にも組み込める。
    https://cutt.ly/uyuTyst0
  • リポジトリ全体を監査 → CSAF(自社プロダクト / ¥4,980 買い切り)
    MISRA 準拠チェックと CWE データフロー解析をリポジトリ横断で走らせ、監査レポート(HTML)を出力。
    レビューの属人化を止めたいチーム向けで、商用の静的解析ツールなら数十万円かかる基本機能を、個人でも入れられる価格にしています。
    https://cutt.ly/5tKoVA2h

無料の2つで「使える」と感じたら、リポジトリ全体に監査を回すのが CSAF です。バージョンは随時更新しています。

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?