Feature/fix zap ZAPのバージョンアップに対応#6619
Conversation
|
nanasess
left a comment
nanasess
left a comment
There was a problem hiding this comment.
ZAPスキャン網羅性レビュー：「確実にスキャンできているか」
全差分を精査した結果、スキャンが正しく実行されない（＝脆弱性を見逃す）可能性がある問題を複数発見しました。
🔴 重大：admin_order_list.zst — POSTデータのURLエンコードエラー（L66）
order%5BShipping%5D%5Bshipping_delivery_date%5D%=
^
ここに余分な % がある
正しくは shipping_delivery_date%5D=（%5D = ]）。
影響: shipping_delivery_date のフィールド名がサーバー側でパースできず、受注登録フォームの バリデーションエラー になります。302リダイレクトではなく200が返るため、次のステップ（index 6）で Location ヘッダから order-id を抽出できません。結果として 受注関連の全スキャン（受注一覧検索、CSV出力、ステータス更新、お問い合わせ番号設定、納品書出力）が無効 になります。
🔴 重大：admin_order_list.zst — indexの欠番（L192, L248）
indexが以下のように飛んでいます：
12 → 14（13が欠番）17 → 19（18が欠番）
Zestエンジンはindex順にステートメントを実行します。欠番があるとスキップまたはエラーになる可能性があり、受注一覧のorder-id抽出（ZestAssignRegexDelimiters）以降のステップが実行されない 恐れがあります。
🟡 中程度：admin_customer_delivery.zst — url と urlToken の不一致（L315-316, L377-378）
"url": "https://ec-cube/admin/customer/1/edit",
"urlToken": "https://ec-cube/admin/customer/{{customer-id}}/delivery/new",url は顧客編集ページ (/edit) を指していますが、urlToken はお届け先新規ページ (/delivery/new) を指しています。これは2箇所で発生しています（index 22, index 26）。
影響: ZAPの sequence-activeScan は urlToken をスキャン対象URLとして使用するため、お届け先新規ページに対してアクティブスキャンが実行されますが、実際にリクエストされるのは顧客編集ページです。スキャン結果のURL情報と実際のリクエスト先が異なるため、脆弱性レポートの信頼性が低下します。
また、お届け先削除のリクエスト（index 25, L355）で urlToken に {{customer-id}} が使われていません：
"urlToken": "https://ec-cube/admin/customer/1/delivery/{{delivery-id}}/delete",ここは customer/{{customer-id}}/delivery/... であるべきです。
🟡 中程度：admin_order_mail.zst — 管理画面ログインステップなし
他のadmin系スクリプトには先頭に管理画面ログイン処理（GET /admin/login → CSRFトークン取得 → POST /admin/login → GET /admin/ 確認）の4ステップが追加されていますが、admin_order_mail.zst にはありません。
代わりに最初のリクエスト（index 1, L15）が GET /admin/customer/new になっています。
ワークフローの before_script で admin_login.zst が実行される設計であることは理解できますが、admin_order_mail はCIのmatrix定義で before_script が明示的に指定されているかを確認する必要があります。もし admin_* パターンのデフォルト割り当てだけに依存している場合、admin_login.zst は standalone タイプとして実行されるため、セッションがtargetスクリプト側に引き継がれるかの検証が必要です。
📋 軽微な指摘
-
admin_order_mail.zstの受注登録POSTデータでorder[Customer]が1にハードコード（L140付近）: 直前のステップで作成した顧客のIDを使うべきではないか？（admin_create_customer.zstのようにcustomer-idを抽出して使用するパターンとの不整合） -
admin_order_list.zstのcsrf1変数名の再利用（index 4 と index 10）: 受注登録フォームのCSRFトークン（order__token）と受注検索フォームのCSRFトークン（admin_search_order__token）が同じ変数名csrf1に格納されています。受注登録POST（index 5）の後で変数が上書きされるので動作上は問題ないかもしれませんが、デバッグ時に混乱の元になります。
✅ 良い変更点
- 全admin系スクリプトへの認証フロー追加（セッション確立の信頼性向上）
entry.zstの\r?\nパターンとZestAssignReplaceによるCR除去対策forceuser.groovyのContextWrapper廃止APIからの移行- 失敗時ログ取得ステップの追加（デバッグ効率大幅向上）
- artifactの名前衝突解消（
overwrite: true+zap-*-alerts分離） jq -s 'add // []'でのマージ処理堅牢化
修正の優先度
| 優先度 | 問題 | ファイル |
|---|---|---|
| 🔴 高 | URLエンコードエラー %5D%= |
admin_order_list.zst L66 |
| 🔴 高 | index欠番 (13, 18) | admin_order_list.zst L192, L248 |
| 🟡 中 | url/urlToken不一致 | admin_customer_delivery.zst L315-316, L377-378 |
| 🟡 中 | ログインステップ欠落 | admin_order_mail.zst |
|
FBいただいた分を修正しています。 以下についてのみ、ワークフロー(zaproxy.yml)内でadmin系に対しては、ログインシーケンスが設定されていましたので、問題無いと判断しています。 修正後、再度ZAPを回り結果が変わっていないことも確認いたしました。 |
nanasess
left a comment
nanasess
left a comment
There was a problem hiding this comment.
再レビュー：指摘事項の修正確認
前回指摘した4点について、最新コミット (660bf3a) で確認しました。
✅ 修正済み
| 指摘 | 状態 |
|---|---|
🔴 admin_order_list.zst URLエンコードエラー %5D%= |
✅ shipping_delivery_date%5D= に修正済み |
🔴 admin_order_list.zst index欠番 (13, 18) |
✅ 1〜25の連番に修正済み |
🟡 admin_customer_delivery.zst url/urlToken不一致 |
✅ 誤った urlToken の除去、および url/urlToken の {{customer-id}} 統一が完了 |
🟡 admin_order_mail.zst ログインステップなし |
✅ zaproxy.yml で admin_* パターンに対し admin_login.zst が before_script として設定されており、問題なし |
📝 軽微な残存事項（ブロッカーではない）
admin_customer_delivery.zst L315 の url が customer/1/edit とハードコードされたままですが、Preprocessステップで {{customer-id}} → 1 に置換される設計のため、実行時には問題ありません。
ZAPの実行結果でも結果が変わっていないことが確認されているとのことで、スキャン網羅性に問題はないと判断します。
|
| コントローラー | パス | mode値 |
|---|---|---|
ContactController |
/contact |
confirm, complete |
EntryController |
/entry |
confirm, complete |
WithdrawController |
/mypage/withdraw |
confirm, complete |
※ ShoppingController (/shopping) は mode ではなく別ルート (/shopping/confirm, /shopping/checkout) で遷移するため対象外
ZAPスクリプトのカバレッジ
| スクリプト | mode=confirm | mode=complete | 状態 |
|---|---|---|---|
front_contact.zst |
✅ index 3 でPOST | ✅ index 5 でPOST | カバー済み |
entry.zst |
✅ index 5 でPOST | ✅ index 7 でPOST | カバー済み |
mypage_change.zst |
✅ mode=confirm 送信 | ✅ mode=complete 送信 | カバー済み |
| (withdraw用) | — | — | スクリプトなし |
sequence-activeScan での mode 遷移の扱い
template.yml の sequence-activeScan は、Zestスクリプトに定義されたリクエストをシーケンスとして順番にリプレイし、各ステップでアクティブスキャンを実行します。
mode=confirmのPOSTリクエスト → フォームの各フィールドにペイロードを注入してスキャンmode=completeのPOSTリクエスト → 先行するconfirmステップの状態を維持した上でスキャン
mode遷移を明示的にZestスクリプトで定義しているため、手動リクエストなしでも正常にスキャンされる設計です。
唯一のギャップ：/mypage/withdraw（退会処理）
WithdrawController は mode=confirm → mode=complete で退会処理を行いますが、対応するZAPスクリプトが存在せず、CIのmatrixターゲットにも含まれていません。退会処理には会員データ論理削除・メール送信・セッション破棄などセキュリティ上重要な処理が含まれます。
これは本PR以前から存在する既存のギャップのため、別途 Issue を作成しました: #6674
結論
本PRの mode 遷移カバレッジに問題はありません。
管理画面の
|
| コントローラー | パス | mode値 | ZAPスクリプト | 状態 |
|---|---|---|---|---|
EditController |
/admin/order/new, /admin/order/{id}/edit |
register |
admin_order_edit.zst, admin_order_list.zst, admin_order_mail.zst |
✅ |
MailController |
/admin/order/{id}/mail |
change, confirm, complete |
admin_order_mail.zst |
✅ |
ShippingController |
/admin/shipping/{id}/edit |
register |
admin_order_mail.zst |
✅ |
FileController |
/admin/content/file_manager |
create, upload, move |
admin_content_file.zst |
✅ |
TaxRuleController |
/admin/setting/shop/tax |
edit_inline |
admin_tax.zst |
✅ |
CalendarController |
/admin/setting/shop/calendar |
edit_inline |
なし | ❌ |
詳細
MailController の3段階遷移（最も複雑）:
admin_order_mail.zst で mode=change（テンプレート変更）→ mode=confirm（確認）→ mode=complete（送信）の全ステップがカバーされています。
FileController のマルチパート送信:
admin_content_file.zst で mode=upload（ファイルアップロード）、mode=create（ディレクトリ作成）、mode=move（移動）がカバーされています。
ギャップ
CalendarController（/admin/setting/shop/calendar）:mode=edit_inlineのスキャンスクリプトがありません。admin_shop_setting.zstは/admin/setting/shopと/admin/setting/shop/csvのみが対象です。（PR Feature/fix zap ZAPのバージョンアップに対応 #6619 以前からの既存ギャップ）
追加発見
admin_order_edit.zst の受注登録POSTデータに shipping_delivery_date%5D%= というURLエンコードエラーがあります（%5D= が正しい）。admin_order_list.zst では本PRで修正済みですが、admin_order_edit.zst は変更対象外のため同じエラーが残っています。
|
@zeniya0000 |
CSRFトークンの動的指定が必要な機能のスキャン状況CSRFトークンを動的に指定する必要がある機能を網羅的に洗い出し、ZAPスクリプトでの実装状況を確認しました。 1. 配列形式CSRFトークン（
|
| コントローラー | フォーム名 | ZAPスクリプト | 抽出方法 | 状態 |
|---|---|---|---|---|
TagController |
tag_{id} |
admin_product_tag.zst |
ZestAssignRegexDelimiters tag_\d+\[_token\] |
✅ |
ClassNameController |
class_name_{id} |
admin_product_class_name.zst |
ZestAssignRegexDelimiters class_name_\d+\[_token\] |
✅ |
ClassCategoryController |
class_category_{id} |
admin_product_class_name.zst |
ZestAssignRegexDelimiters class_category_\d+\[_token\] |
✅ |
CategoryController |
category_{id} |
admin_product_category.zst |
ZestAssignRegexDelimiters category_\d+\[_token\] |
✅ |
TaxRuleController |
tax_rule（固定名） |
admin_tax.zst |
ZestAssignFieldValue form[0] |
✅ |
CalendarController |
calendar（固定名） |
なし | — | ❌ (#6675) |
2. AJAXリクエスト（ECCUBE-CSRF-TOKEN ヘッダー）
<meta name="eccube-csrf-token"> からトークンを取得し、ECCUBE-CSRF-TOKEN HTTPヘッダーとして送信するパターンです。
| 機能 | ZAPスクリプト | 状態 |
|---|---|---|
タグ並び替え (sort_no/move) |
admin_product_tag.zst |
✅ |
| 規格名並び替え | admin_product_class_name.zst |
✅ |
| 規格分類並び替え | admin_product_class_name.zst |
✅ |
| カテゴリ並び替え | admin_product_category.zst |
✅ |
| 配送方法並び替え | admin_delivery.zst |
✅ |
| 支払方法並び替え | admin_payment.zst |
✅ |
| 商品画像アップロード/削除 | admin_product_edit.zst |
✅ |
| 支払方法画像UP/削除 | admin_payment.zst |
✅ |
| 受注: 会員検索/商品検索 | admin_order_edit_search.zst |
✅ |
| 受注ステータス更新 | admin_order_list.zst |
✅ |
| お問い合わせ番号設定 | admin_order_list.zst |
✅ |
| メールプレビュー | admin_mail.zst |
✅ |
3. 削除リンク（_token + _method=delete）
csrf_token_for_anchor() でトークンを埋め込み、_token={{eccube-csrf-token}}&_method=delete としてPOSTするパターンです。お知らせ、ブロック、レイアウト、ページ、ファイル、テンプレート、お届け先、会員、メンバー、配送方法、支払方法、税率、規格分類、規格名の削除が該当し、すべてカバー済みです。
結論
配列形式CSRFトークン、AJAXヘッダー、削除リンクの3パターンとも、ZAPスクリプトで適切に実装されています。唯一のギャップは CalendarController（#6675）のみです。
修正後の該当スクリプトの診断結果に変更はありませんでした。 |
|
4 participants
概要(Overview・Refs Issue)
OSSのGitHub Actionsで使用している OWASP ZAP のバージョンが古いため、最新バージョンでスキャンが実行されるよう環境の刷新を行いました。
変更内容
最新バージョンの利用に伴い、依存関係およびイメージの管理方法を以下の通り変更しています。
・セキュリティスキャンの精度向上と最新の脆弱性への対応のため。
・上記ZAPのアップデートに伴い、動作要件を満たすJDKバージョンへ更新。
・外部リポジトリから取得していたイメージを、ec-cube 本体のGitHub（GitHub Packages）内へ移行
動作確認
事前に以下の個人リポジトリにて動作確認を行っています。
https://github.com/zeniya0000/ec-cube/actions/runs/22991096107/job/66752310528
マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目
Summary by CodeRabbit
新機能
バグ修正
Chores