reviewing-security

セキュリティレビュー

概要

セキュリティレビューの出力のほとんどは、パスではなくカテゴリを挙げているため役に立ちません。「すべてのユーザー入力を検証する」は、これまでに書かれたあらゆるプログラムに当てはまり、何も変えません。有用な指摘とは、パスです。つまり、攻撃者が制御する入力、それを運ぶコード、その入力が到達する操作、そして攻撃者が得るものです。

基本原則： チェックリストではなく、データをたどってください。脆弱性は、値が入力される場所と作用する場所の間に潜んでいます。

関連スキル： 一般的なレビューの手順については requesting-code-review、エクスプロイトのパスを確認するときは systematic-debugging を参照してください。

鉄則

NO FINDING WITHOUT A PATH FROM ATTACKER-CONTROLLED INPUT TO IMPACT

入力、通る経路、そして運用者に生じるコストを特定できない場合、それはコードのにおいです。「観察事項」に分類し、「指摘」には含めないでください。カテゴリに基づく助言で報告を水増しすると、実在する唯一のパスが埋もれてしまいます。

その帰結： 報告する前に、実際のコードでパスをたどってください。grep のヒットは手がかりであって、指摘ではありません。

チェックリスト

1. 攻撃対象領域を把握する — エントリーポイント、到達できるユーザー、実行時の権限
2. 入力からシンクまでたどる — あらゆる形態のインジェクション
3. 認可 — 最も有効性の高い確認項目
4. シークレットと暗号
5. プロセスとプラグインの境界
6. 依存関係
7. データの露出 — ログ、エラー、レスポンス
8. スキャナーを実行する — その後、報告する前に到達可能性を検証する
9. 報告する — エクスプロイト可能性に基づいて深刻度を判断し、それぞれにパスを示す

ステップ 1：攻撃対象領域を把握する

実装を読む前にこれを行ってください。見知らぬユーザーが到達できる場所の一覧を作成します。

エントリーポイント	到達できるユーザー	実行時の権限
HTTP ルート、gRPC メソッド	匿名／認証済み／管理者／内部のみ	プロセス、DB ロール、ファイルシステム
キューまたはイベントのコンシューマー	発行できるすべてのユーザー	通常はプロセス全体
ファイルのアップロード、インポート	任意の認証済みユーザー	パーサーはプロセス内で実行
Webhook レシーバー	署名が検証されるまではインターネット上の誰でも
CLI フラグ、環境変数、設定	ローカルの運用者
LLM／ツールの出力	モデルのコンテキストに影響を与えられるすべてのユーザー	ツールが実行できるすべての操作

# Enumerate handlers and consumers — adapt the pattern to the framework
grep -rnE '\.(Get|Post|Put|Patch|Delete|Handle|HandleFunc)\(' --include='*.go' --include='*.ts'
grep -rnE 'func \(.*\) [A-Z][A-Za-z]*\(ctx context.Context' --include='*.go'   # gRPC impls

最後の行は目新しいものではありません。モデルの出力は信頼できない入力です。 ドキュメント、Web ページ、または以前のツール実行結果によって、モデルがツールを呼び出すよう誘導できるなら、そのツールが到達するすべてのシンクは攻撃者から到達可能です。ツールの定義は、HTTP ハンドラーと同じように警戒してレビューしてください。

ステップ 2：入力からシンクまで追跡する

各エントリーポイントについて、値を追跡します。シンクとは、その値が悪意あるものである場合に動作が変わる操作です。

シンク	危険となる要因	安全な形式
SQL／クエリビルダー	文字列連結または文字列補間	パラメーター化クエリ。パラメーター化できない識別子には許可リストを使用
シェル／プロセスの起動	シェルインタープリター、または argv[0] のユーザーデータ	シェルを使わず、argv 配列と固定のバイナリパスで exec
ファイルシステムパス	..、絶対パス、シンボリックリンク、NUL	結合し、filepath.EvalSymlinks を適用してから、結果がルート配下にあることを確認
アウトバウンド HTTP	ユーザー指定 URL → SSRF	ホストを許可リストに登録。プライベート、リンクローカル、メタデータの範囲をブロックし、リダイレクト後に再確認
テンプレートのレンダリング	自動エスケープが無効、または dangerouslySetInnerHTML	デフォルトでエスケープし、実績のあるサニタイザーで HTML をサニタイズ
デシリアライズ	ポリモーフィック形式、または型情報を含む形式	固定スキーマを使い、具象型にデコードして、サイズと深さに上限を設定
アーカイブの展開	zip-slip、シンボリックリンク、展開爆弾	各エントリーパスを検証し、合計バイト数とエントリー数に上限を設定
リダイレクト先	オープンリダイレクト → フィッシング、トークン窃取	パスを許可リストに登録するか、同一オリジンのみに限定
ユーザー入力に対する正規表現	壊滅的なバックトラッキング	入力長に上限を設け、ネストした量指定子を避ける
eval／リフレクション／動的インポート	任意コードの実行	安全な形式はありません。削除してください

境界を越えて値を追跡してください。 よくある見落としは、値を境界で検証して保存した後、読み戻して信頼してしまうことです。保存してもサニタイズにはなりません。後でレンダリングまたは実行する場合、依然として汚染された値です。

各行の具体的な脆弱な例と修正例については、references/vulnerability-patterns.md を参照してください。

ステップ 3：認可

本当のバグがあるのはここです。 インジェクションは最新のライブラリで十分に防御されていますが、オブジェクトレベル認可の不備はそうではありません。どの行が誰に属するかを把握する必要があるため、フレームワークでは修正できません。

• ID を受け取るすべてのハンドラーで、呼び出し元がログインしているだけでなく、そのオブジェクトにアクセスできることを確認する
• チェックはデータが実際に通る経路上で行う。サービス層が迂回できるルート層のチェックは、チェックとはいえない
• テナント／組織のスコープはクエリ内（WHERE tenant_id = ?）に指定し、取得後の結果に適用しない
• リクエストボディからロール、テナント、価格、ユーザー ID を信頼しない。マスアサインメントは、ひと手間加わった認可の回避である
• デフォルトで拒否する。明示的なポリシーのない新しいルートにはアクセスできず、公開されない
• 状態を変更する操作に GET でアクセスできない
• 連番 ID だけが、ユーザーと他人のレコードとの間を隔てている状態にしない
• 管理者向けの機能は、リンクのない URL であること以上の方法で制限する
• 複数ステップのフローでは、開始時だけでなく各ステップで認可を再確認する

# Handlers that take an identifier — each one needs an ownership check
grep -rnE 'Param\("id"\)|params\.id|req\.body\.(userId|tenantId|role)' --include='*.go' --include='*.ts'

ステップ 4：シークレットと暗号技術

• ソースコード、Git にコミットされた設定、ログ、エラーメッセージ、URL、クライアントから見えるバンドルにシークレットを含めない
• Git の 履歴 にシークレットを残さない。一度でもコミットしたものはローテーションする。後のコミットで削除しても何も解決しない
• パスワードは argon2id／scrypt／bcrypt でハッシュ化する。単純な SHA は使わず、ソルトなしも不可
• トークンと署名は、定時間比較関数で比較する
• トークン、ID、ソルト用の乱数は CSPRNG（crypto/rand、crypto.randomUUID）から取得し、math/rand は使わない
• JWT：署名を検証し、アルゴリズムを固定して（none とアルゴリズム混同を拒否）、exp／aud／iss をチェックする
• TLS 検証を無効にしない。InsecureSkipVerify: true も NODE_TLS_REJECT_UNAUTHORIZED=0 も使わない
• 権限の変更時にセッショントークンをローテーションし、ログアウト時にはサーバー側で無効化する
• Cookie：HttpOnly、Secure、SameSite を意図的に設定する
• 暗号プリミティブを独自に実装したり、カスタムの「暗号化」を作成したりしない

gitleaks detect --no-banner          # or: trufflehog git file://.
git log -p -S'BEGIN PRIVATE KEY' --oneline | head

ステップ 5：プロセスとプラグインの境界

サブプロセスを起動するもの、プラグインを読み込むもの、またはモデルに代わってツールを実行するものについては、次の点を確認してください。

• 子プロセスに継承させるのは必要な環境変数だけです。すべての認証情報を含む親プロセスの環境全体を渡してはいけません。
• 作業ディレクトリ、開いているファイル記述子、ネットワークアクセスは、意図して設定されています。デフォルトで継承させてはいけません。
• 子プロセスの出力はデータとして解析し、信頼できる指示として再入力してはいけません。
• タイムアウト、メモリ、出力サイズなどのリソース制限が設定されています。制限のない子プロセスは、サービス拒否攻撃の手段になります。
• フェイルクローズになっています。プラグインがクラッシュしたり応答しなくなったりした場合、チェックをスキップするのではなく、操作を拒否します。
• ツールのインターフェース自体がセキュリティ境界です。パス、URL、コマンドを受け取るツールはシンクであり、モデルに影響を与える者に公開されています。

ステップ 6：依存関係

• ロックファイルがコミットされ、ビルドで適切に使用されています（npm ci、go mod verify）。
• アドバイザリースキャンに問題がないか、例外ごとに到達可能性の根拠を含む説明が文書で記録されています。
• 新しい直接依存関係について、公開からの期間、保守状況、ダウンロード数を確認し、タイポスクワッティングを狙った名前でないか確認します。
• 新しい依存関係のインストールスクリプトと postinstall スクリプトを確認します。
• CI のアクションは移動するタグではなく、SHA に固定されています。

govulncheck ./...          # Go — reports only vulnerabilities you actually reach
npm audit --omit=dev       # Node
pip-audit                  # Python

govulncheck は、到達可能性の分析を行うため、汎用スキャナーよりも有用です。脆弱な関数が呼び出されているかどうかを教えてくれるツールを優先してください。

ステップ 7：データの漏えい

• クライアントに返すエラーに、スタックトレース、SQL、ファイルパス、内部ホスト名が含まれていません。
• ログに、パスワード、トークン、セッション ID、カード番号全体、必要以上の個人識別情報（PII）が含まれていません。
• デバッグ用エンドポイント（/debug/pprof、GraphQL のイントロスペクション、管理コンソール）は、本番環境で無効化されているか、アクセスが制限されています。
• CORS：Access-Control-Allow-Origin: * と認証情報の送信が組み合わされていません。許可するオリジンのリストが明示されています。
• 認証済みのレスポンスは、共有キャッシュでキャッシュされません。
• オブジェクトストレージのバケットとその一覧は、デフォルトで非公開になっています。
• タイミングやエラーメッセージの違いから、アカウントの有無が明らかになりません。

ステップ 8：スキャナーを実行してから検証する

semgrep --config=auto .

スキャナーの出力は調査の手掛かりの一覧であり、レポートではありません。スキャナーが検出した問題を報告する前に、必ずこのコードベースで到達可能であることを確認してください。 シンクが実際に汚染された値で呼び出されているかどうかを確認します。ツールの生の出力をそのまま転送すると、レビューは無視されるノイズになり、実際の問題まで埋もれてしまいます。

重大度

カテゴリ名ではなく、悪用可能性に基づいて評価してください。

重大度	定義
Critical	認証されていない攻撃者が、コード実行、認証情報、または他のユーザーのデータへの大量アクセスを得ます。特別な前提条件はありません。
High	認証済みの攻撃者が権限を昇格させる、別のテナントのデータを読み書きする、またはシークレットを取得します。
Medium	特殊な前提条件（特定の競合状態、複数のバグの連鎖、ユーザーの操作）が必要であるか、取得できるデータが限定的です。
Low	強化に関する問題です。実際の弱点はありますが、現時点で実証された攻撃経路はありません。将来のバグを封じ込める、多層防御の欠落です。

重大度を誇張しないでください。すべてが Critical のレポートでは、誰も対応しません。

レポートの形式

## Scope
[What was reviewed — diff range, paths, or "full repo". What was NOT reviewed.]
[Scanners run and their versions.]

## Critical
**Tenant isolation missing on document fetch**
`internal/api/documents.go:88` — the handler loads by primary key only:
`db.First(&doc, c.Param("id"))`. There is no `tenant_id` predicate and no
post-fetch ownership check.
**Path:** any authenticated user → `GET /api/documents/{id}` → sequential
integer IDs → full document body of any tenant.
**Impact:** cross-tenant read of all documents; IDs are enumerable.
**Fix:** scope the query — `db.Where("tenant_id = ?", claims.TenantID).First(&doc, id)`
— and add the predicate at the repository layer so new callers inherit it.

## High
[same shape]

## Medium / Low
[same shape]

## Observations (not findings)
[Weaknesses with no demonstrated path. Say so plainly rather than promoting them.]

## Verified Clean
[What you checked and found sound. Silence reads as "not checked".]

## Assessment
**Ship?** Yes / No / After the Critical and High items
**Reasoning:** [1-2 sentences]

報告してはいけないもの

報告してはいけないもの	理由
シンクを示さずに「入力検証を追加する」とだけ述べること	カテゴリに基づく助言であり、問題の指摘ではありません。
悪用可能なエンドポイントを挙げずにレート制限の欠如を指摘すること	エンドポイントとそのコストを示すか、指摘を取り下げてください。
ブラウザクライアントがないサービスの、ブラウザーセキュリティヘッダーの欠落	その脅威は該当しません。
テスト、フィクスチャ、サンプルコードにある問題	攻撃者から到達できません。
到達可能性を確認していないスキャナーの出力	実際の問題を埋もれさせるノイズです。
脅威の裏付けがない、暗号方式の好みに関する細かな指摘	「AES-128 ではなく AES-256 を使う」は問題の指摘ではありません。
「理論上」と表現するような指摘	悪用可能なら、攻撃経路を示せるはずです。

よくある間違い

パスのレビューではなく、チェックリストの確認をすること。 OWASP Top 10を上から順に確認すると、フレームワークですでに対処されている脆弱性を見つける一方、このドメイン特有の認可バグを見落とします。

エントリーポイントで確認を終えること。 エッジでの検証だけでは、その値が保存され、後でクエリに連結される場合、何の保証にもなりません。

フレームワークを盲信すること。 ORMには生クエリを使う抜け道があり、テンプレートには生の出力を使う抜け道があります。どちらも実際に使われています。これらを対象に明示的にgrepしてください。

認証済みであれば認可済みと見なすこと。 実世界で最もよく見つかる問題であり、クエリではなくルートテーブルを読むと最も見落としやすい問題です。

モデルを信頼できるものとして報告すること。 モデルが読み取ったコンテンツによってツール呼び出しが左右される可能性があるなら、その引数は攻撃者に制御されています。

徹底しているように見せるため、重大度を過大評価すること。 最も重要な指摘に割く読者の注意を損ないます。