From 0d5430d72a80342e1f20d1d7a9927bd2869d98b8 Mon Sep 17 00:00:00 2001 From: Mathieu Ducrot Date: Wed, 23 Sep 2026 08:14:32 +0200 Subject: [PATCH] v1.7.1 DAST scan gate with failOnRisk, alert filters baseline, ascanrulesBeta and Qualys parity scan rules --- CHANGELOG.md | 44 ++++++++ dast_scan/build-plan.sh | 44 ++++++++ dast_scan/scripts/blank-target-links.js | 94 +++++++++++++++ dast_scan/scripts/client-side-cookies.js | 95 ++++++++++++++++ dast_scan/scripts/form-autocomplete.js | 114 +++++++++++++++++++ dast_scan/scripts/phpinfo.js | 2 +- docs/security.md | 138 +++++++++++++++++++---- make/security.mk | 20 +++- 8 files changed, 528 insertions(+), 23 deletions(-) create mode 100644 dast_scan/scripts/blank-target-links.js create mode 100644 dast_scan/scripts/client-side-cookies.js create mode 100644 dast_scan/scripts/form-autocomplete.js diff --git a/CHANGELOG.md b/CHANGELOG.md index ce8618f..375897e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,49 @@ CHANGELOG for 1.x =================== +## v1.7.1 - (2026-09-23) +### TODO When updating +- Run `composer recipes:install smartbooster/standard-bundle --reset --force` to pick up the updated `make/security.mk`, + `dast_scan/build-plan.sh` and `dast_scan/scripts/` (the set of copied files is unchanged, so there is no new dotted manifest) +- The DAST scans now **fail the build** as soon as an alert reaches `failOnRisk` (default `Low`), where they used to always exit `0` : + before enabling `dast-blackbox-ci` on a project that already ran it, either arbitrate the current alerts in `dast_scan/alert-filters.json` + or raise `failOnRisk` in the project's `dast_scan/targets.json` ; see [docs/security.md](docs/security.md) +- Widen `reportRisks` in `dast_scan/targets.json` to at least the levels covered by `failOnRisk` (`["info", "low", "medium", "high"]` with + the default), otherwise the CI fails on alerts absent from the reports +- The first run of each `dast-*` target now downloads the `ascanrulesBeta` add-on into the container (set `DAST_ADDONS=` to opt out) + +### Fixed + +All the following is added content but it aim to fix the overall behavior of zap scan tool chain. + +- `dast_scan/build-plan.sh` : `exitStatus` job appended to every generated plan, the gate that turns an alert into a red pipeline + (ZAP itself always exits `0`). `errorLevel` comes from the new `failOnRisk` key of `targets.json` (default `Low`), `warnLevel` is + `Informational`, and `alwaysRun: true` keeps the status set when an earlier job interrupts the plan +- `dast_scan/build-plan.sh` : optional `dast_scan/alert-filters.json` baseline, rendered as an `alertFilter` job emitted before any + traffic-generating job (a filter only applies to the alerts raised after it is registered). An alert requalified as `False Positive` + is skipped by `exitStatus` while staying visible in the report ; the non-ZAP `note` key of each filter is rendered as a YAML comment + above the filter it documents, so the generated plan carries the arbitration. Like `targets.json`, the file is project-specific and + **never shipped by the recipe** +- Three custom passive scan rules in `dast_scan/scripts/`, covering vulnerabilities Qualys reports and ZAP has no native equivalent for : + - `form-autocomplete.js` (id `9000001`, QID 150112) : `type="password"` input with autocompletion disabled neither on the field nor on + its form — ZAP retired its own rule `10012` in 2018, Qualys still reports it + - `blank-target-links.js` (id `9000002`, QID 150222) : cross-domain `target="_blank"` link without `rel="noopener"` — the native rule + `10108` only alerts when `rel` explicitly contains `opener`, so a link with no `rel` at all never triggers it, at any threshold + - `client-side-cookies.js` (id `9000003`, QID 150122/150123) : cookie written by the served JavaScript — it never appears in a + `Set-Cookie` header, so the native rules `10010`/`10011` are structurally blind to it +- `make/security.mk` : `DAST_ADDONS` variable (default `ascanrulesBeta`), installing the listed add-ons into the container before the scan. + The official image ships the release add-ons only, and the CORS active scan rule (alert `40040`, equivalent of Qualys' QID 150631) lives + in beta. The install is a **separate `zap.sh` invocation** in the same container : doing it in the `-autorun` run pulls a newer + `commonlib` and breaks the custom scripts (`Could not initialize class ...ScanRuleMetadata`) +- `make/security.mk` : ZAP's `zap.log` copied out of the container into `dast_scan/report/.log` before `--rm` discards it. In + `-cmd` mode the log never reaches stdout, so it is the only way to read the warnings counted by the report's `insight.log.warn` ; the + copy is unconditional and the exit code saved beforehand, so a failing scan still yields it +- `docs/security.md` : "The gate (`failOnRisk`)", "The baseline (`dast_scan/alert-filters.json`)" (schema, the `String.matches()` whole-URL + trap, the silence of a filter matching nothing) and "Add-ons (`DAST_ADDONS`)" sections ; the custom rules section now lists the four + shipped rules and reserves the `9000001+` id range for home-made ones + +### Changed +- `dast_scan/scripts/phpinfo.js` : id `100001` → `9000004`, to sit in the `9000001+` range now reserved for the bundle's own scan rules + ## v1.7.0 - (2026-09-21) ### Added - PHPUnit extension `DeprecationLoggerExtension` and `DeprecationLoggerSubscriber` to intercept and log unhandled deprecations during test execution diff --git a/dast_scan/build-plan.sh b/dast_scan/build-plan.sh index aaf63a3..21ff44c 100755 --- a/dast_scan/build-plan.sh +++ b/dast_scan/build-plan.sh @@ -20,12 +20,18 @@ # excludePaths : regexes taken out of scope in active mode, for the routes that are # destructive or that trigger emails. # +# Two more files shape the plan: +# dast_scan/alert-filters.json : the baseline, turned into an alertFilter job so that an +# arbitrated alert stops failing the build while staying in the report. +# dast_scan/scripts/*.js : home-made scan rules, inlined below. +# set -eu mode=${1:?"missing mode (passive|crawl|active)"} env_name=${2:?"missing environment (see .environments in targets.json)"} targets=${3:-"$(dirname "$0")/targets.json"} scripts_dir="$(dirname "$0")/scripts" +filters_file="$(dirname "$0")/alert-filters.json" command -v jq >/dev/null 2>&1 || { echo "build-plan.sh: jq is required" >&2; exit 1; } [ -f "$targets" ] || { echo "build-plan.sh: targets file not found ($targets)" >&2; exit 1; } @@ -43,6 +49,7 @@ spider_max_mins=$(jq -r --arg env "$env_name" '.environments[$env].spiderMaxMins ajax_max_mins=$(jq -r --arg env "$env_name" '.environments[$env].spiderAjaxMaxMins // 2' "$targets") active_max_mins=$(jq -r --arg env "$env_name" '.environments[$env].activeScanMaxMins // 10' "$targets") report_risks=$(jq -r --arg env "$env_name" '.environments[$env].reportRisks // [] | join(", ")' "$targets") +fail_on_risk=$(jq -r --arg env "$env_name" '.environments[$env].failOnRisk // "Low"' "$targets") prefix=${4:-"blackbox-$env_name-$mode"} context="blackbox" @@ -107,6 +114,30 @@ jobs: enableTags: false EOF +# The baseline, first of all the jobs that matter: an alert filter only applies to the alerts +# raised after it is registered, so it has to come before any job generating traffic. +# "note" is ours, not ZAP's: it is rendered as a YAML comment above the filter it documents. +alert_filters() { + [ -f "$filters_file" ] || return 0 + jq -r ' + .filters[]? as $f + | [$f | del(.note) | to_entries[]] as $entries + | (if ($f.note // "") != "" then " # " + $f.note else empty end), + ( range(0; $entries | length) as $i + | (if $i == 0 then " - " else " " end) + + $entries[$i].key + ": " + ($entries[$i].value | tojson) ) + ' "$filters_file" +} + +if [ -n "$(alert_filters)" ]; then + cat <<'EOF' + + - type: alertFilter + alertFilters: +EOF + alert_filters +fi + # Home-made passive scan rules, registered before any job that generates traffic. # The code is copied into the plan (inline) rather than mounted into the container: the plan # stays self-contained, and the one archived by the CI holds the rules that actually ran. @@ -235,3 +266,16 @@ EOF # It weighs about 7 times more than the standard template. report_job traditional-html-plus report_job traditional-json + +# The gate. ZAP itself exits 0 whatever it finds, so this job is what turns an alert into a red +# pipeline. Alerts requalified as "False Positive" by the alertFilter job are skipped by ZAP here +# (ExitStatusJob ignores CONFIDENCE_FALSE_POSITIVE), which is what makes the baseline work. +# alwaysRun so that the status is still set when an earlier job interrupts the plan. +cat < carrying no rel at all - the exact pattern +// Qualys reports as QID 150222 - never triggers it, at any threshold. + +const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRuleMetadata"); + +function getMetadata() { + return ScanRuleMetadata.fromYaml(` +id: 9000002 +name: Link to another domain opened with target="_blank" and no rel="noopener" +description: > + The page links to another domain with target="_blank" but without rel="noopener" nor + rel="noreferrer". The opened page can then reach the opener through window.opener and + redirect the original tab, for instance to a phishing page (reverse tabnabbing). +solution: > + Add rel="noopener noreferrer" to every link carrying target="_blank". +references: + - https://cwe.mitre.org/data/definitions/1022.html +risk: LOW +confidence: HIGH +cweId: 1022 +wascId: 15 +alertTags: + QUALYS_QID: "150222" +status: alpha +`); +} + +const ANCHOR_PATTERN = /<(?:a|area)\b[^>]*>/gi; + +function attribute(tag, name) { + const match = new RegExp(name + '\\s*=\\s*("([^"]*)"|\'([^\']*)\'|([^\\s">]+))', "i").exec(tag); + if (match === null) { + return null; + } + const value = match[2] !== undefined ? match[2] : (match[3] !== undefined ? match[3] : match[4]); + return value === undefined ? null : value; +} + +function hostOf(url) { + const match = /^https?:\/\/([^/?#]+)/i.exec(url); + return match === null ? null : match[1].toLowerCase(); +} + +function scan(ps, msg, src) { + if (!msg.getResponseHeader().isHtml()) { + return; + } + + const host = String(msg.getRequestHeader().getHostName()).toLowerCase(); + const body = String(msg.getResponseBody().toString()); + const reported = {}; + + ANCHOR_PATTERN.lastIndex = 0; + let match; + while ((match = ANCHOR_PATTERN.exec(body)) !== null) { + const tag = match[0]; + + const target = (attribute(tag, "target") || "").toLowerCase(); + if (target !== "_blank") { + continue; + } + + const rel = (attribute(tag, "rel") || "").toLowerCase(); + if (rel.indexOf("noopener") !== -1 || rel.indexOf("noreferrer") !== -1) { + continue; + } + + // Relative links stay on the same origin: only absolute links to another host matter. + const href = attribute(tag, "href"); + if (href === null) { + continue; + } + const linkHost = hostOf(href); + if (linkHost === null || linkHost === host) { + continue; + } + + // One alert per target URL is enough, a menu can repeat the same link many times. + if (reported[href]) { + continue; + } + reported[href] = true; + + ps.newAlert() + .setParam(href) + .setEvidence(tag) + .setOtherInfo("Qualys reports this as QID 150222 (CWE-1022).") + .setMessage(msg) + .raise(); + } +} diff --git a/dast_scan/scripts/client-side-cookies.js b/dast_scan/scripts/client-side-cookies.js new file mode 100644 index 0000000..135f8b3 --- /dev/null +++ b/dast_scan/scripts/client-side-cookies.js @@ -0,0 +1,95 @@ +// Passive scan rule: cookies written by client-side JavaScript. +// +// A cookie written through document.cookie never appears in a Set-Cookie response header, so +// the native rules 10010 (HttpOnly) and 10011 (Secure) are structurally blind to it - as is any +// scanner that only looks at responses. Qualys sees those cookies because its crawler reads them +// from the browser, and reports them as QID 150122 (no Secure) and QID 150123 (no HttpOnly). +// +// This rule works on what is always observable: the JavaScript actually served. A cookie written +// there can never be HttpOnly, and its Secure attribute depends solely on client code, so it is +// worth an explicit review - and the rule fires whether or not the crawl happened to trigger the +// write, which is what makes it reliable. +// +// Only writes naming the cookie are reported. A bare "document.cookie = someVariable" is the +// plumbing of every cookie library and would alert on each bundle embedding one. A name built +// from a template literal - cookies.set(`${s}`, ...), as shipped by swagger-ui - is the same +// case: what the regex captures is the template, not a cookie name. + +const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRuleMetadata"); + +function getMetadata() { + return ScanRuleMetadata.fromYaml(` +id: 9000003 +name: Cookie written by client-side JavaScript +description: > + The served JavaScript writes a cookie from the browser. Such a cookie cannot carry the HttpOnly + attribute (it would break the code reading it back), and its Secure attribute is set by the + client code alone, not by the server: both have to be reviewed in the code rather than assumed. +solution: > + Keep client-side cookies free of any sensitive value, and make sure the code setting them forces + the Secure attribute when the page is served over HTTPS. Move to a server-set cookie whenever the + value does not have to be read by JavaScript. +references: + - https://cwe.mitre.org/data/definitions/1004.html + - https://cwe.mitre.org/data/definitions/614.html +risk: LOW +confidence: HIGH +cweId: 1004 +wascId: 13 +alertTags: + QUALYS_QID: "150122 / 150123" +status: alpha +`); +} + +// cookies.set("name", ...) / $cookies.set('name') / Cookies.set(`name`) - the call sites of the +// usual libraries (vue-cookies, js-cookie...), which survive minification. +const LIBRARY_WRITE_PATTERN = /(?:\$?cookies|cookieStore)\s*\.\s*set\s*\(\s*["'`]([^"'`]+)["'`]/gi; + +// document.cookie = "name=..." - the raw write, when the name is a literal. +const RAW_WRITE_PATTERN = /document\s*\.\s*cookie\s*=\s*["'`]([^="'`]+)=/gi; + +function isJavaScript(msg) { + const contentType = msg.getResponseHeader().getHeader("Content-Type"); + return contentType !== null && String(contentType).toLowerCase().indexOf("javascript") !== -1; +} + +// A ${...} left in the captured text means the name is assembled at runtime: nothing to report. +function isDynamic(name) { + return name.indexOf("${") !== -1; +} + +function collect(body, pattern, names) { + pattern.lastIndex = 0; + let match; + while ((match = pattern.exec(body)) !== null) { + const name = match[1].trim(); + if (name.length > 0 && !isDynamic(name) && names.indexOf(name) === -1) { + names.push(name); + } + } +} + +function scan(ps, msg, src) { + if (!msg.getResponseHeader().isHtml() && !isJavaScript(msg)) { + return; + } + + const body = String(msg.getResponseBody().toString()); + const names = []; + collect(body, LIBRARY_WRITE_PATTERN, names); + collect(body, RAW_WRITE_PATTERN, names); + + for (let i = 0; i < names.length; i++) { + ps.newAlert() + .setParam(names[i]) + .setEvidence(names[i]) + .setOtherInfo( + "Cookie written from JavaScript, so it can carry neither HttpOnly (Qualys QID 150123) " + + "nor a server-controlled Secure attribute (Qualys QID 150122). Check the code that " + + "sets it." + ) + .setMessage(msg) + .raise(); + } +} diff --git a/dast_scan/scripts/form-autocomplete.js b/dast_scan/scripts/form-autocomplete.js new file mode 100644 index 0000000..0ea5f3e --- /dev/null +++ b/dast_scan/scripts/form-autocomplete.js @@ -0,0 +1,114 @@ +// Passive scan rule: sensitive form field left with browser autocompletion enabled. +// +// ZAP retired its own rule (10012, "Password Autocomplete in browser") in pscanrules 22 +// (2018, issue 4215) because browsers ignore autocomplete="off" on password fields. +// Qualys still reports it as QID 150112, category Vulnerabilities, so the rule is kept here +// for audit parity: the point is to be warned before an external audit is, not to claim the +// finding is exploitable. +// +// A field is considered covered when either the field itself or its enclosing form carries +// an autocomplete value that turns the feature off. +// +// Only type="password" is reported. Guessing sensitivity from the field name was tried and +// dropped: a Symfony form scopes its field names, so the plain e-mail input of the "forgot +// password" form is named forgot_password[email] and matched every "password" heuristic. + +const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRuleMetadata"); + +function getMetadata() { + return ScanRuleMetadata.fromYaml(` +id: 9000001 +name: Sensitive form field has not disabled autocomplete +description: > + A password input is served without an autocomplete attribute that turns browser autocompletion + off, neither on the field nor on its enclosing form. On a shared workstation the browser may + offer to store and replay the value. +solution: > + Add autocomplete="off" (or "new-password" / "current-password" where a password manager is + wanted) to the field, or autocomplete="off" to the form that contains it. +references: + - https://cwe.mitre.org/data/definitions/200.html +risk: LOW +confidence: HIGH +cweId: 200 +wascId: 15 +alertTags: + QUALYS_QID: "150112" +status: alpha +`); +} + +const FORM_OPEN_PATTERN = /]*>/gi; +const INPUT_PATTERN = /]*>/gi; + +// Values that actually disable the browser's own storage of the value. +const SAFE_VALUES = ["off", "new-password", "current-password"]; + +function attribute(tag, name) { + const match = new RegExp(name + '\\s*=\\s*("([^"]*)"|\'([^\']*)\'|([^\\s">]+))', "i").exec(tag); + if (match === null) { + return null; + } + const value = match[2] !== undefined ? match[2] : (match[3] !== undefined ? match[3] : match[4]); + return value === undefined ? null : value; +} + +function isSensitive(tag) { + return (attribute(tag, "type") || "").toLowerCase() === "password"; +} + +function autocompleteDisabled(tag) { + const value = (attribute(tag, "autocomplete") || "").toLowerCase(); + return SAFE_VALUES.indexOf(value) !== -1; +} + +// Splits the body into segments, each one carrying the
tag it belongs to (null outside +// of any form), so that an autocomplete="off" set on the form covers the fields it contains. +function segments(body) { + const result = []; + let lastIndex = 0; + let lastForm = null; + + FORM_OPEN_PATTERN.lastIndex = 0; + let match; + while ((match = FORM_OPEN_PATTERN.exec(body)) !== null) { + result.push({ form: lastForm, html: body.substring(lastIndex, match.index) }); + lastForm = match[0]; + lastIndex = FORM_OPEN_PATTERN.lastIndex; + } + result.push({ form: lastForm, html: body.substring(lastIndex) }); + + return result; +} + +function scan(ps, msg, src) { + if (!msg.getResponseHeader().isHtml()) { + return; + } + + const body = String(msg.getResponseBody().toString()); + const parts = segments(body); + + for (let i = 0; i < parts.length; i++) { + const part = parts[i]; + if (part.form !== null && autocompleteDisabled(part.form)) { + continue; + } + + INPUT_PATTERN.lastIndex = 0; + let match; + while ((match = INPUT_PATTERN.exec(part.html)) !== null) { + const tag = match[0]; + if (!isSensitive(tag) || autocompleteDisabled(tag)) { + continue; + } + + ps.newAlert() + .setParam(attribute(tag, "name") || attribute(tag, "id") || "") + .setEvidence(tag) + .setOtherInfo("Qualys reports this as QID 150112 (CWE-200).") + .setMessage(msg) + .raise(); + } + } +} diff --git a/dast_scan/scripts/phpinfo.js b/dast_scan/scripts/phpinfo.js index 326e287..a23af94 100644 --- a/dast_scan/scripts/phpinfo.js +++ b/dast_scan/scripts/phpinfo.js @@ -10,7 +10,7 @@ const ScanRuleMetadata = Java.type("org.zaproxy.addon.commonlib.scanrules.ScanRu function getMetadata() { return ScanRuleMetadata.fromYaml(` -id: 100001 +id: 9000004 name: Exposed phpinfo() output description: > The response contains the output of the PHP phpinfo() function. It discloses the PHP diff --git a/docs/security.md b/docs/security.md index 4f34547..4ad85b9 100644 --- a/docs/security.md +++ b/docs/security.md @@ -11,12 +11,13 @@ Scans are **blackbox**: no authentication, only the public routes of the project ## What the recipe ships -| File | Role | -|----------------------------|----------------------------------------------------------------------------------------------------| -| `make/security.mk` | the `dast-*` make targets, the Docker run and the `DAST_*` variables | -| `dast_scan/build-plan.sh` | generates the ZAP plan (contexts, jobs, reports) from `targets.json`, on standard output | -| `dast_scan/scripts/*.js` | custom passive scan rules, injected inline into every generated plan (phpinfo response detection) | -| `dast_scan/targets.json` | **not shipped — each project writes its own** (see [Setup](#setup-mandatory-after-a-recipe-reset)) | +| File | Role | +|--------------------------------|--------------------------------------------------------------------------------------------------------------------------| +| `make/security.mk` | the `dast-*` make targets, the Docker run and the `DAST_*` variables | +| `dast_scan/build-plan.sh` | generates the ZAP plan (contexts, jobs, reports) from `targets.json`, on standard output | +| `dast_scan/scripts/*.js` | custom passive scan rules, injected inline into every generated plan (see [Custom passive rules](#custom-passive-rules)) | +| `dast_scan/targets.json` | **not shipped — each project writes its own** (see [Setup](#setup-mandatory-after-a-recipe-reset)) | +| `dast_scan/alert-filters.json` | **not shipped — optional, each project writes its own** (see [The baseline](#the-baseline-dast_scanalert-filtersjson)) | ## Setup (mandatory after a recipe reset) @@ -45,6 +46,8 @@ Then complete the setup: `build-plan.sh` to read `targets.json`). 3. Make sure the application answers on the declared `baseUrl` (the container runs with `--network host`, so `http://{sub}.localhost` works against a local stack). +4. Optionally write `dast_scan/alert-filters.json`, the project's arbitrated baseline — see + [The baseline](#the-baseline-dast_scanalert-filtersjson). Absent, the plan simply carries no `alertFilter` job. ## `dast_scan/targets.json` @@ -62,7 +65,8 @@ Then complete the setup: "spiderMaxMins": 5, "spiderAjaxMaxMins": 2, "activeScanMaxMins": 30, - "reportRisks": ["medium", "high"] + "reportRisks": ["info", "low", "medium", "high"], + "failOnRisk": "Low" } }, "sites": [ @@ -104,6 +108,7 @@ One entry per environment, addressed by the make target (`local`, `ci`, …). An | `spiderAjaxMaxMins` | `2` | cap, in minutes, of the AJAX spider, **per site** having `ajaxSpider: true` | | `activeScanMaxMins` | `10` | cap, in minutes, of the active scanner (`active` mode) | | `reportRisks` | all | risk levels kept in the reports, e.g. `["medium", "high"]` to drop `info`/`low` noise | +| `failOnRisk` | `Low` | lowest risk making the build fail, see [The gate](#the-gate-failonrisk) — `High`, `Medium`, `Low` or `Informational` | ### `sites` @@ -128,6 +133,68 @@ Exclude anything an attack payload must not reach: destructive routes (`.*/delet (`.*/forgot_password.*`, `.*/inscription.*`), payment callbacks (`.*/payment-ipn.*`), the profiler (`.*/_wdt/.*`, `.*/_profiler.*` when using it on dev mode) and the logout, which would only kill the session. +## The gate (`failOnRisk`) + +ZAP **always exits `0`**, whatever it finds. What turns an alert into a red pipeline is the `exitStatus` job, appended to every generated +plan with `errorLevel` set to the environment's `failOnRisk` (default `Low`) and `warnLevel` to `Informational`. It is declared +`alwaysRun: true`, so the status is still set when an earlier job interrupts the plan. + +Two consequences worth knowing before raising the bar: + +- `failOnRisk` filters on the **risk**, not on the report: an alert dropped from the reports by `reportRisks` still fails the build if its + risk reaches `failOnRisk`. Keep `reportRisks` at least as wide as `failOnRisk`, otherwise the CI fails on an alert that appears nowhere. +- the job skips every alert requalified as `False Positive` (ZAP's `ExitStatusJob` ignores `CONFIDENCE_FALSE_POSITIVE`) — which is exactly + what makes the baseline below work. + +## The baseline (`dast_scan/alert-filters.json`) + +A scan that fails on `Low` only stays useful if the alerts already arbitrated stop failing it. `dast_scan/alert-filters.json` is that +baseline: `build-plan.sh` turns it into an `alertFilter` job, emitted **before** any job generating traffic (a filter only applies to the +alerts raised after it is registered). The file is optional and, like `targets.json`, **never shipped by the recipe**: it holds project +arbitrations, and a shipped example would overwrite the project's own file on every reset. + +```json +{ + "filters": [ + { + "ruleId": "10096", + "url": "https://admin\\.[^/]+(/.*)?", + "urlRegex": true, + "newRisk": "False Positive", + "note": "Timestamp Disclosure on admin: the integers are the Sonata asset cache-busters (?v=1767630660), not a server clock." + }, + { + "ruleId": "40040-2", + "url": "https://api\\.[^/]+(?:/(?!doc).*)?", + "urlRegex": true, + "newRisk": "Info", + "note": "QID 150631 - outside /doc, reflecting the Origin is accepted: the api is stateless (Bearer token, no session cookie)." + } + ] +} +``` + +| Key | Role | +|----------------------------------|---------------------------------------------------------------------------------------------------------------------------------------| +| `ruleId` | rule id, or `alertRef` to target a single alert of a multi-alert rule (`10055-4` is one variant of the CSP rule, `10055` all of them) | +| `newRisk` | `False Positive` (skipped by the gate, still visible in the report) or `Info` / `Low` / `Medium` / `High` to requalify | +| `url` + `urlRegex` | restrict the filter to an URL, literal or regex | +| `parameter` + `parameterRegex` | restrict it to a parameter (cookie name, field name…) | +| `attack`/`evidence` (+ `…Regex`) | restrict it to the payload sent or to the evidence found | +| `methods`, `context` | restrict it to HTTP methods or to a ZAP context | +| `note` | **ours, not ZAP's**: stripped from the job and rendered as a YAML comment above the filter it documents | + +Two traps: + +- ZAP matches with `String.matches()`, so a regex must describe the **whole** URL. `"https://api\\.[^/]+/(?!doc)"` filters nothing; + `"https://api\\.[^/]+/(?!doc).*"` does (JSON, hence the doubled backslash). +- a filter that matches nothing is **silent** — no warning in the report, no line in the log. After adding one, re-run the scan and check + the alert really moved. + +Prefer scoping a filter by what makes it a false positive (the parameter, the evidence, the rule) rather than by the URL that happened to +raise it: a filter pinned on URLs silently stops covering the next route, while a filter pinned on the rule keeps the arbitration explicit. +Always fill `note` — it ends up in the generated plan, which is the artefact the next audit reads. + ## Commands Three levels of increasing invasiveness, each one including the previous: @@ -148,14 +215,27 @@ make dast-plan DAST_MODE=active DAST_ENV=ci > `active` submits forms and sends attack payloads: it can create accounts, trigger emails and write junk data. +## Add-ons (`DAST_ADDONS`) + +The official ZAP image ships the **release** add-ons only. `DAST_ADDONS` (default `ascanrulesBeta`) lists the extra ones installed into the +container before each run — `ascanrulesBeta` is what brings the CORS active scan rule (alert `40040`, the equivalent of Qualys' QID 150631), +along with `40025` *Proxy Disclosure* and `40038` *Bypassing 403*. Add-on ids are **case sensitive**; set `DAST_ADDONS=` to install none. + +The install runs as a **separate `zap.sh` invocation** inside the same container, before the one doing `-autorun`. Installing in the same run +does not work: resolving `ascanrulesBeta` pulls a newer `commonlib`, and swapping it while ZAP is up breaks the custom scripts that build +their metadata from it (`Could not initialize class …ScanRuleMetadata`). + ## Output -Every run writes two things, both named after the make target: +Every run writes three things, all named after the make target: - the plan actually executed, in `dast_scan/generated/.yaml` — regenerated on each run, useful to check the scope before attacking and archivable by the CI; - the reports, in `dast_scan/report/-.html` and `.json` (`traditional-html-plus` and `traditional-json` templates), - restricted to the declared sites and to `reportRisks`. + restricted to the declared sites and to `reportRisks`; +- ZAP's own log, copied out of the container into `dast_scan/report/.log`. In `-cmd` mode the log never reaches stdout, so this is + the only way to read the warnings counted by the report's `insight.log.warn`. The copy is unconditional (the exit code is saved + beforehand), so a **failing** scan — the case where the log matters most — still yields it. The `-plus` HTML template also lists the rules that ran **without** finding anything, the statistics, the parameters and the request/response of each alert instance: the report says what was analysed, not only what alerted. @@ -166,20 +246,36 @@ request/response of each alert instance: the report says what was analysed, not plan stays self-contained, and the one archived by the CI holds the rules that actually ran. They are registered before any traffic-generating job and enabled in the three modes. -`phpinfo.js` is the shipped example: it detects a `phpinfo()` output in **any** response body, where the native *Hidden File Finder* rule -only probes four known paths (`phpinfo.php`, `info.php`, `i.php`, `test.php`). +Four are shipped. The last three exist because Qualys reports vulnerabilities ZAP has no native equivalent for: the point is to be warned by +our own CI before an external audit is, hence the `QUALYS_QID` alert tag carried by each of them. + +| Script | Id | What it catches | +|--------------------------|-----------|-----------------------------------------------------------------------------------------------------------------------| +| `phpinfo.js` | `9000004` | a `phpinfo()` output in **any** response body, where the native *Hidden File Finder* rule only probes four known paths (`phpinfo.php`, `info.php`, `i.php`, `test.php`) | +| `form-autocomplete.js` | `9000001` | a `type="password"` input whose autocompletion is disabled neither on the field nor on its form (QID 150112). ZAP retired its own rule `10012` in 2018 because browsers ignore `autocomplete="off"` on passwords; Qualys still reports it | +| `blank-target-links.js` | `9000002` | a cross-domain link with `target="_blank"` and no `rel="noopener"` (QID 150222). The native rule `10108` only alerts when `rel` explicitly contains `opener`, so a link with no `rel` at all never triggers it, at any threshold | +| `client-side-cookies.js` | `9000003` | a cookie written by the served JavaScript (QID 150122 / 150123). Such a cookie never appears in a `Set-Cookie` header, so the native rules `10010`/`10011` — and any scanner reading responses only — are structurally blind to it | + +The last three deliberately report from the **served code** rather than from observed traffic: they fire whether or not the crawl happened to +reach the page, which is what makes them reliable in CI. Each carries in its header the false positives that were tried and dropped +(name-based heuristics on Symfony scoped field names, `document.cookie = someVariable` in every cookie library…) — read it before widening a +pattern. To add one, drop a `.js` file in `dast_scan/scripts/` — `build-plan.sh` picks up every file of the directory automatically. It must export a -`getMetadata()` returning a `ScanRuleMetadata.fromYaml(...)` (with an `id` that collides with no existing rule — `phpinfo.js` uses `100001`) -and a `scan(ps, msg, src)` raising the alert. +`getMetadata()` returning a `ScanRuleMetadata.fromYaml(...)` and a `scan(ps, msg, src)` raising the alert. Take the next id in the +`9000001+` range reserved here for home-made rules, well clear of the ids ZAP's own add-ons use. ## Troubleshooting -| Symptom | Cause / fix | -|------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------| -| `build-plan.sh: targets file not found` | `dast_scan/targets.json` was not written after the recipe reset (see [Setup](#setup-mandatory-after-a-recipe-reset)) | -| `build-plan.sh: unknown environment (x). Known: …` | the make target asks for an environment absent from `.environments` | -| `build-plan.sh: no route to replay` | `paths` is empty, or `excludePaths` excludes all of them in `active` mode | -| `make: *** [dast-…] Error 137` | the JVM got `SIGKILL`ed: increase the value of the `DAST_JVM_MEM` variable | -| `AccessDeniedException` on the reports | the report directory must be writable by uid 1000 (the target already `chmod 777` it, check a stale root-owned dir) | -| ZAP finds nothing but the pages exist | check the `baseUrl` from **inside** the container (`--network host`), and that the routes are really public | +| Symptom | Cause / fix | +|----------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------| +| `build-plan.sh: targets file not found` | `dast_scan/targets.json` was not written after the recipe reset (see [Setup](#setup-mandatory-after-a-recipe-reset)) | +| `build-plan.sh: unknown environment (x). Known: …` | the make target asks for an environment absent from `.environments` | +| `build-plan.sh: no route to replay` | `paths` is empty, or `excludePaths` excludes all of them in `active` mode | +| `make: *** [dast-…] Error 137` | the JVM got `SIGKILL`ed: increase the value of the `DAST_JVM_MEM` variable | +| `AccessDeniedException` on the reports | the report directory must be writable by uid 1000 (the target already `chmod 777` it, check a stale root-owned dir) | +| ZAP finds nothing but the pages exist | check the `baseUrl` from **inside** the container (`--network host`), and that the routes are really public | +| `make: *** [dast-…] Error 1` with no error in the output | nominal: the `exitStatus` job found an alert at or above `failOnRisk`. Read the report, then fix it or arbitrate it in `alert-filters.json` | +| a filter of `alert-filters.json` changes nothing | the regex must match the **whole** URL (`String.matches()`); a filter matching nothing is silent. Check the emitted job with `make dast-plan` | +| `Could not initialize class …ScanRuleMetadata` | an add-on was installed in the same `zap.sh` run as the scan, swapping `commonlib` under the scripts — keep the two invocations separate | +| an alert is absent from the report but fails the build | `reportRisks` is narrower than `failOnRisk` (see [The gate](#the-gate-failonrisk)) | diff --git a/make/security.mk b/make/security.mk index 4ff34c8..74b73a6 100644 --- a/make/security.mk +++ b/make/security.mk @@ -22,11 +22,29 @@ DAST_REPORT_DIR ?= dast_scan/report # to zap.sh takes precedence over that computation. DAST_JVM_MEM ?= 2g +# The official image ships the release add-ons only: the CORS active scan rule (Zap Alert Id 40040, equivalent of the 150631 from +# Qualys) lives in ascanrulesBeta, which is downloaded into the container. Add-on ids are case sensitive. +DAST_ADDONS ?= ascanrulesBeta + +# Installing in the same run as -autorun does not work: resolving ascanrulesBeta pulls a newer +# commonlib, and swapping it while ZAP is up breaks the scripts that build their metadata from it +# ("Could not initialize class ...ScanRuleMetadata"). Hence two invocations in the same container: +# the first one installs, the second one starts with everything already in place. +ifeq ($(strip $(DAST_ADDONS)),) +DAST_ADDON_CMD := +else +DAST_ADDON_CMD := zap.sh -cmd $(foreach addon,$(DAST_ADDONS),-addoninstall $(addon)) && +endif + # Generates the plan, named after the make target, then has ZAP execute it. # $(1) = mode, $(2) = environment declared in targets.json. # The chmod is there for the CI: ZAP runs under uid 1000 and writes its reports into the # mounted volume, which the runner creates as root (AccessDeniedException otherwise). The # plans directory stays untouched, it is mounted read-only. +# zap.log is copied out of the container before it is discarded (--rm): in -cmd mode the log +# never reaches stdout, so it is the only way to read the warnings counted by the report's +# "insight.log.warn". The copy runs after a ";" and not a "&&", and the exit code is saved +# beforehand, so a failing scan - the case where the log matters most - still yields it. define dast_run @mkdir -p $(CURDIR)/$(DAST_PLAN_DIR) $(CURDIR)/$(DAST_REPORT_DIR) @chmod 777 $(CURDIR)/$(DAST_REPORT_DIR) @@ -35,7 +53,7 @@ define dast_run docker run --rm --network host \ -v $(CURDIR)/$(DAST_PLAN_DIR):/zap/plans:ro \ -v $(CURDIR)/$(DAST_REPORT_DIR):/zap/wrk:rw \ - $(DAST_IMAGE) zap.sh -Xmx$(DAST_JVM_MEM) -cmd -autorun /zap/plans/$@.yaml + $(DAST_IMAGE) sh -c '$(DAST_ADDON_CMD) zap.sh -Xmx$(DAST_JVM_MEM) -cmd -autorun /zap/plans/$@.yaml; RC=$$?; cp -f /home/zap/.ZAP/zap.log /zap/wrk/$@.log 2>/dev/null; exit $$RC' endef .PHONY: dast-blackbox-passive dast-blackbox-crawl dast-blackbox-active dast-blackbox-ci dast-plan