🛡️

テストが全部緑でも壊せる｜リリース前アドバーサリアルレビューを常設する

に公開
PHP
Security
http
process
idea

はじめに

テストも型チェックも CI も全部緑なのに、「このリリース、本当に大丈夫か」という不安が消えない方へ。

テストが全部グリーンで、型チェックも通って、CI も真っ青。それでも「このリリース、本当に大丈夫か？」という不安が残ることがある。この不安の正体は、テストが確認しているのが「仕様どおり動くか」であって「壊せないか」ではない、というズレだ。

正しい入力に対して正しく動くことは、テストがいくらでも保証してくれる。しかし攻撃者は正しい入力を送ってこない。負の Content-Length を送り、ヘッダに改行を混ぜ、Origin: * のような「常識的にはありえない」値を投げてくる。この領域はハッピーパスのテストからは構造的に漏れる。

そこで私たちが自作の PHP フレームワーク NENE2 で常設しているのが、リリースごとに一度、自分のコードを自分で攻めるアドバーサリアルレビューだ。この記事では、その仕組みと、実際にそのレビューで自分たちが見つけた 2 件のバグ（size limit のバイパスと HTTP ヘッダインジェクション）を紹介する。どちらも「テストは通っていたが壊せた」典型例だ。

この記事で分かること：

  • ハッピーパスのテストが構造的に見逃すバグの種類
  • Content-Length バイパスと WWW-Authenticate CRLF 注入の実例と修正
  • アドバーサリアルレビューをリリースゲートに常設する型

アドバーサリアル観点を常設する

アドバーサリアルレビューを「気が向いたらやる」タスクにすると、忙しいリリース前ほど真っ先に飛ばされる。だから NENE2 では ADR（Architecture Decision Record）にリリースプロセスの一部として明文化した（ADR 0011: Security Review Policy）。

観点はシンプルに 3 つの前提を置くだけだ。

  • デフォルト設定は誤設定されていると仮定する — 「普通そう設定しないでしょ」は攻撃者には通じない。
  • すべてのパラメータに攻撃者が制御する入力が届くと仮定する — ヘッダ、クエリ、ボディ、例外メッセージまで。
  • バイパス・インジェクション・情報漏洩の経路を探す — 「動くか」ではなく「すり抜けられるか」を問う。

見つかったものは深刻度で仕分けする。

深刻度 内容
P1 クリティカル／悪用可能なバイパス
P2 インジェクション／権限昇格
P3 フットガン／多層防御の穴

そして重要なのが、見つけたバイパス・インジェクション・フットガンは、修正をマージする前に必ず専用テストで覆うというルール（ADR 0011 §4）。テスト名にセキュリティ意図を込めるので、後から読んでも「なぜこのテストがあるのか」が消えない。実際、後述する 2 件はそれぞれ testRequestSizeLimitRejectsNegativeContentLength と testWwwAuthenticateStripsCrlfFromDescription という名前で残っている。

以下、この観点で実際に自分たちが掘り当てた 2 件を見ていく。

発見例①: Content-Length: -1 による size limit バイパス

NENE2 にはリクエストボディのサイズ上限を課す RequestSizeLimitMiddleware がある。上限を超えたら 413 Payload Too Large を返す、という素直なミドルウェアだ。当然「上限を超えたら 413」というテストは通っていた。

問題は Content-Length ヘッダの値を検証する部分だった。当時の判定はこうだった。

private function isOversized(string $contentLength): bool
{
    if (preg_match('/\A\d+\z/', $contentLength) !== 1) {
        return false; // ← 数字でなければ「oversized ではない」とみなして通す
    }

    return (int) $contentLength > $this->maxBodyBytes;
}

「10 進の非負整数でなければ false（＝上限超過ではない）」としていた。ここに攻撃者の視点を当てるとどうなるか。

Content-Length: -1 を送る。-1 は \A\d+\z（- を含まない数字列）にマッチしないので isOversized() は false を返す。ミドルウェアは「上限超過ではない」と判断してリクエストをそのまま下流に流す。しかも process() の構造上、Content-Length ヘッダが存在する場合はこの分岐で処理が完結し、ボディを実測するフォールバック（ヘッダが空のときだけ走る経路）には到達しない。つまり -1（や abc、0x10、小数など）を付けるだけで、上限チェックもボディ実測も両方すり抜けて、いくらでも大きいペイロードを送り込めた。

修正は 1 行、判定を反転させるだけだ。

private function isOversized(string $contentLength): bool
{
    if (preg_match('/\A\d+\z/', $contentLength) !== 1) {
        // 非整数の Content-Length（負値・小数・16進等）は oversized として拒否。
        // RFC 9110 §8.6 は非負10進整数を要求する。それ以外は全て拒否。
        return true;
    }

    return (int) $contentLength > $this->maxBodyBytes;
}

パースできない Content-Length は「安全側に倒して拒否」する。RFC 9110 §8.6 が Content-Length を非負 10 進整数と定めている以上、それ以外の値は 413 で弾いて正しい。

再現条件と対策を専用テストで固定した。

public function testRequestSizeLimitRejectsNegativeContentLength(): void
{
    // ...maxBodyBytes = 10 のミドルウェア...
    $request = $factory
        ->createServerRequest('POST', 'https://example.test/upload')
        ->withHeader('Content-Length', '-1');

    $response = $middleware->process($request, $this->okHandler($factory));

    self::assertSame(413, $response->getStatusCode(), 'Negative Content-Length must be rejected.');
}

-1 だけでなく abc のような非数値も 413 になることを別テスト（testRequestSizeLimitRejectsNonNumericContentLength）で覆っている。

発見例②: WWW-Authenticate への CRLF 注入

もう 1 件は認証まわり。BearerTokenMiddleware はトークン検証に失敗すると 401 を返し、その際 WWW-Authenticate ヘッダに失敗理由を載せる。当時のコードはこうだった。

->withHeader(
    'WWW-Authenticate',
    sprintf('Bearer realm="NENE2", error="%s", error_description="%s"', $error, $description),
);

この $description は、トークン検証時に投げられる例外のメッセージ（TokenVerificationException::getMessage()）から来る。ここで「すべてのパラメータに攻撃者制御の入力が届くと仮定する」観点が効く。例外メッセージは、検証対象のトークンの中身に応じて動的に生成されうる。つまり攻撃者がある程度内容を左右できるデータが、無検証でレスポンスヘッダに埋め込まれていた。

これは 2 系統の穴になる。

  1. HTTP レスポンス分割 / ヘッダインジェクション: メッセージに \r\n（CRLF）が混ざると、ヘッダが途中で切れて別のヘッダを注入できる。例えばメッセージが Line1\r\nX-Injected: evil なら、X-Injected: evil という攻撃者由来のヘッダがレスポンスに紛れ込む。
  2. quoted-string の破壊: メッセージに " が含まれると error_description="..." の引用符が閉じてしまい、ヘッダ構造が壊れる。

修正は、ヘッダに載せる値を必ず無害化してから埋め込むこと。

private function sanitizeHeaderParam(string $value): string
{
    // CRLF を除去（ヘッダインジェクション対策）してから、
    // RFC 7235 quoted-string に従いダブルクォートをエスケープする。
    return str_replace('"', '\\"', preg_replace('/\r?\n|\r/', ' ', $value) ?? $value);
}

\r / \n をスペースに潰し、残った " を \" にエスケープする。呼び出し側は error_description="%s" に渡す前にこれを通す。

CRLF 除去のほうは、攻撃者由来の改行を含む例外メッセージを投げてもヘッダに改行が残らないことをテストで固定した。

public function testWwwAuthenticateStripsCrlfFromDescription(): void
{
    $verifier = new class () implements TokenVerifierInterface {
        public function verify(string $token): array
        {
            throw new TokenVerificationException("Line1\r\nX-Injected: evil");
        }
    };
    // ...
    $header = $response->getHeaderLine('WWW-Authenticate');
    // レスポンス分割を防ぐため CRLF は必ず除去されていること。
    self::assertStringNotContainsString("\r\n", $header);
    self::assertStringNotContainsString("\n", $header);
}

ダブルクォートのエスケープ側も別テスト（testWwwAuthenticateEscapesDoubleQuotesInDescription）で、"sub" のような値が \"sub\" にエスケープされ、error_description の引用符が途中で閉じないことを確認している。

この 2 件を「HTTP セキュリティ不変条件」として ADR に昇格させたのがポイントだ。単発の修正で終わらせず、全ミドルウェアが守るべきルールとして明文化した。

  • Content-Length は非負 10 進整数でなければならない（RFC 9110 §8.6）。それ以外は oversized 扱い。
  • レスポンスヘッダに載せる全ての値は \r\n を除去し、埋め込む " をエスケープする（RFC 7230 §3.2 / RFC 7235）。

こうしておけば、新しいミドルウェアを追加したときも同じチェックリストで殴れる。

リリースゲートへの組み込み方

アドバーサリアルレビューを「常設」する肝は、誰が・いつ・何を試すかを固定することだ。属人的な「気づき」に頼らない。NENE2 では次のようにリリースゲートに埋め込んでいる。

  • いつ: マイナー／メジャーリリースごとに 1 回、リリース前に必ず 1 パス。
  • 何を: 前述の 3 前提（デフォルトは誤設定・全パラメータに攻撃者入力が届く・バイパス/インジェクション/情報漏洩を探す）を、ミドルウェア横断のチェックリストとして適用する。
  • 記録: 見つけたものは深刻度 P1/P2/P3 で GitHub Issue 化して追跡する。
  • 修正の受け入れ条件: 発見したバイパス・インジェクション・フットガンは、修正をマージする前に専用テストで覆う。テスト名にセキュリティ意図を込めて、意図が風化しないようにする。
  • 設計判断は ADR に残す: 「なぜこのヘッダを無害化するのか」「なぜ Content-Length: -1 を 413 にするのか」を ADR に不変条件として書き、次のミドルウェアにも適用できるようにする。

ここで効いてくる副次的な設計方針が 2 つある。1 つはフェイルファストなコンストラクタガード。誤設定は本番のリクエスト時に静かに壊れるのではなく、構築時に例外を投げてユニットテストで落とす。もう 1 つはログの無条件化。ErrorHandlerMiddleware は $debug フラグに関わらず全ての未処理例外を記録する。$debug はレスポンス本文に例外メッセージを出すかどうかだけを制御し、サーバ側ログの有無は制御しない。攻撃の痕跡を本番で必ず観測できるようにするためだ。

このレビューの型は、同じフレームワーク上で他のセキュリティ課題にも横展開している。たとえば FILTER_VALIDATE_URL だけでは塞ぎ切れない SSRF を本気で塞ぐ話（CGN レンジや IPv4-mapped IPv6、DNS rebinding の TOCTOU まで潰す）や、振込名義に =cmd|'/c calc'!A1 が載る現実に対する CSV formula injection の型ベース中和など、いずれも「攻撃者入力が届く前提で殴る」同じ観点から出てきたものだ（これらは別記事として準備中）。

学び

一番の学びは、シンプルに言えばこれだ。

「動く」ことをいくら確認しても、「壊せない」ことは保証されない。

RequestSizeLimitMiddleware も BearerTokenMiddleware も、ハッピーパスのテストは全部通っていた。上限を超えれば 413 を返したし、正しいトークンは通した。それでも -1 一発でサイズ制限を無視でき、例外メッセージ経由でヘッダを割れた。この 2 つは、正常系のテストからは構造的に出てこない。攻撃者の視点で入力を選ばない限り、永遠にグリーンのまま埋もれる種類のバグだ。

だからこそ「攻める」レビューを気分ではなくプロセスとして常設する価値がある。ポイントを 3 つに畳むとこうなる。

  1. 前提を反転させる: 「正しく使われる」ではなく「誤設定されている・攻撃者入力が届く」を出発点に置く。
  2. 発見をテストと ADR に固定する: 単発修正で終わらせず、専用テスト（意図の込もった名前）と不変条件（ADR）に落とし、次のコードにも効かせる。
  3. リリースゲートに埋め込む: 誰が・いつ・何を試すかを決め打ちして、忙しいときほど飛ばされないようにする。

自分のコードを自分で攻めるのは気が進まない作業だ。しかし、リリース後に他人に攻められて気づくよりはずっといい。

一次資料

  • NENE2 ADR 0011: Security Review Policy — Adversarial Review and HTTP Security Invariants（docs/adr/0011-security-review-policy.md）
  • src/Middleware/RequestSizeLimitMiddleware.php（isOversized()）／tests/Middleware/BaselineMiddlewareTest.php（testRequestSizeLimitRejectsNegativeContentLength ほか）
  • src/Auth/BearerTokenMiddleware.php（sanitizeHeaderParam()）／tests/Auth/BearerTokenMiddlewareTest.php（testWwwAuthenticateStripsCrlfFromDescription ほか）
  • 修正 PR: #534（Issue #528 Content-Length バイパス・#529 WWW-Authenticate インジェクションを両方修正）。#535 は同じセキュリティレビュー由来の兄弟修正（CorsMiddleware の maxAge・#532/#533）

シリーズ/相互リンク

  • セキュリティ: （PHP で SSRF を本気で塞ぐ・準備中）・（CSV formula injection を型ベースで中和・準備中）。公開時にリンクを追加する。

── 筆者: 森 秀之（彩音インターナショナル）— 自己ホストの業務ツール群を実運用中。
中小企業向けの業務システムを料金公開・固定価格で開発しています。
🔗 ayane.co.jp

Discussion