From ba40087d944de5e7bb5a4630b427e1b64352652e Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Fri, 12 Jun 2026 21:17:00 +0400 Subject: [PATCH 1/3] fix(lab2): shorten Threagile model title to fit Excel 31-char sheet-name limit Threagile uses the model title as the risks.xlsx sheet name; the shipped 41-char title crashed report generation at the Excel step ('the sheet name length exceeds the 31 characters limit'), leaving JSONs/diagrams but no risks.xlsx or report.pdf. Verified full output with threagile/threagile:0.9.1. Also documented the pitfall in lab2.md. Signed-off-by: Dmitrii Creed --- labs/lab2.md | 1 + labs/lab2/threagile-model.yaml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/labs/lab2.md b/labs/lab2.md index 02fc18ea0..30fdf1b07 100644 --- a/labs/lab2.md +++ b/labs/lab2.md @@ -369,6 +369,7 @@ PR checklist body: - 🚨 **`docker: invalid reference format`** — make sure you wrote `threagile/threagile:0.9.1` not `threagile:v0.9.1` (no namespace). - 🚨 **Output directory empty after run** — Threagile needs write access. Verify the volume mount `-v "$(pwd)/labs/lab2":/app/work` and that `output/` exists with write perms before running. - 🚨 **`undefined protocol: xyz`** — Threagile validates protocol enums. Common typo: `JDBC-encrypted` (capitalized) — use lowercase `jdbc-encrypted`. +- 🚨 **`the sheet name length exceeds the 31 characters limit`** — Threagile uses your model's `title:` as the Excel sheet name in `risks.xlsx`; Excel caps sheet names at 31 characters. Keep `title:` short (≤ 31 chars). The run dies at the Excel step, so you get JSONs and diagrams but no `risks.xlsx`/`report.pdf`. (The `Fontconfig error` lines are harmless noise — ignore them.) - 🚨 **PDF is huge / slow to open** — that's normal. Use `risks.json` + `jq` for fast iteration; open the PDF only for the final report. - 🚨 **Secure variant has MORE risks than baseline** — usually means you added a new asset without declaring its security requirements. Threagile rules can fire on new assets you accidentally introduced; review your diff carefully. - 🚨 **"My auth-flow model has 50 risks!"** — that's usually because you copied the baseline model and trimmed it. Build the auth model **from scratch** — minimum viable assets + links + data. Threagile rules multiply on under-specified models. diff --git a/labs/lab2/threagile-model.yaml b/labs/lab2/threagile-model.yaml index 85c01a799..6d7ca4e76 100644 --- a/labs/lab2/threagile-model.yaml +++ b/labs/lab2/threagile-model.yaml @@ -1,6 +1,6 @@ threagile_version: 1.0.0 -title: OWASP Juice Shop — Local Lab Threat Model +title: OWASP Juice Shop Threat Model date: 2025-09-18 author: From 44f7579e48d04e5feca0828bd7167e15a2bf6f09 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Tue, 23 Jun 2026 22:10:50 +0400 Subject: [PATCH 2/3] fix(lab5): juice-shop hostname, OOM cap, compare_zap params Signed-off-by: Dmitrii Creed --- labs/lab5.md | 5 ++++- labs/lab5/scripts/compare_zap.sh | 12 ++++-------- labs/lab5/scripts/zap-auth.yaml | 26 +++++++++++++------------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/labs/lab5.md b/labs/lab5.md index e80954eff..c6a65e44f 100644 --- a/labs/lab5.md +++ b/labs/lab5.md @@ -97,7 +97,9 @@ docker run --rm --network lab5-net \ ```bash # The provided zap-auth.yaml drives the Automation Framework +# _JAVA_OPTIONS caps ZAP's JVM heap; without it the active scan OOM-kills the container docker run --rm --network lab5-net \ + -e _JAVA_OPTIONS="-Xmx512m" \ -v "$(pwd)/labs/lab5:/zap/wrk" \ ghcr.io/zaproxy/zaproxy:stable \ zap.sh -cmd -autorun /zap/wrk/scripts/zap-auth.yaml -port 8090 @@ -369,7 +371,8 @@ PR checklist body:
⚠️ Common Pitfalls -- 🚨 **`zap-auth.yaml` "context not found"** — the YAML hardcodes Juice Shop running at `juice-shop:3000` (Docker network internal name). If you renamed the container or didn't put both on the same `lab5-net` network, ZAP can't reach it. +- 🚨 **`zap-auth.yaml` "context not found"** — the YAML targets `juice-shop:3000` (Docker network internal name). If you renamed the container or didn't attach both to the same `lab5-net` network, ZAP can't reach it. The container name in `docker run --name` must match exactly. +- 🚨 **Active scan dies silently (CPU 100% → container exit)** — ZAP's active scan is memory-hungry. The `_JAVA_OPTIONS="-Xmx512m"` flag in step 5.3 caps the JVM heap; omitting it lets ZAP consume all available RAM until the container is OOM-killed with no error message. - 🚨 **Auth scan finds 0 alerts** — the `loginRequestBody` in `zap-auth.yaml` ships with the default Juice Shop admin creds (`admin@juice-sh.op` / `admin123`). If you changed them, auth scan logs in as anonymous and only sees unauth surface. - 🚨 **`zap.sh -port 8090` instead of default 8080** — added in the plumbing because the previous lab version conflicted with users running things on 8080. Don't change it unless you also change the YAML. - 🚨 **Semgrep `Parse error: ...`** — Juice Shop's TS sources occasionally hit edge cases. Add `--exclude='**/test/**'` to skip test fixtures if a single parse error blocks the whole scan. diff --git a/labs/lab5/scripts/compare_zap.sh b/labs/lab5/scripts/compare_zap.sh index a32cbec09..f3bd7ee1a 100755 --- a/labs/lab5/scripts/compare_zap.sh +++ b/labs/lab5/scripts/compare_zap.sh @@ -3,10 +3,10 @@ set -e -NOAUTH="labs/lab5/zap/zap-report-noauth.json" -AUTH="labs/lab5/zap/zap-report-auth.json" -OUT="labs/lab5/analysis/zap-comparison.txt" -mkdir -p labs/lab5/analysis +NOAUTH="${1:-labs/lab5/results/baseline-report.json}" +AUTH="${2:-labs/lab5/results/auth-report.json}" +OUT="${3:-labs/lab5/results/zap-comparison.txt}" +mkdir -p "$(dirname "$OUT")" parse_report() { local file="$1" @@ -29,8 +29,6 @@ by_risk = {'3': 0, '2': 0, '1': 0, '0': 0} risk_names = {'3': 'High', '2': 'Medium', '1': 'Low', '0': 'Info'} for site in sites: - if 'localhost:3000' not in site.get('@name', ''): - continue for alert in site.get('alerts', []): risk = alert.get('riskcode', '0') by_risk[risk] = by_risk.get(risk, 0) + 1 @@ -43,8 +41,6 @@ for code in ['3','2','1','0']: # count unique URLs scanned urls = set() for site in sites: - if 'localhost:3000' not in site.get('@name', ''): - continue for alert in site.get('alerts', []): for inst in alert.get('instances', []): urls.add(inst.get('uri', '')) diff --git a/labs/lab5/scripts/zap-auth.yaml b/labs/lab5/scripts/zap-auth.yaml index 0501fffa6..1cbd7e14e 100644 --- a/labs/lab5/scripts/zap-auth.yaml +++ b/labs/lab5/scripts/zap-auth.yaml @@ -2,9 +2,9 @@ env: contexts: - name: "Juice Shop Auth" urls: - - "http://localhost:3000" + - "http://juice-shop:3000" includePaths: - - "http://localhost:3000.*" + - "http://juice-shop:3000.*" excludePaths: - ".*\\.js$" - ".*\\.css$" @@ -15,8 +15,8 @@ env: authentication: method: "json" parameters: - loginPageUrl: "http://localhost:3000/rest/user/login" - loginRequestUrl: "http://localhost:3000/rest/user/login" + loginPageUrl: "http://juice-shop:3000/rest/user/login" + loginRequestUrl: "http://juice-shop:3000/rest/user/login" loginRequestBody: '{"email":"{%username%}","password":"{%password%}"}' verification: method: "poll" @@ -24,7 +24,7 @@ env: loggedOutRegex: ".*\"error\".*" pollFrequency: 60 pollUnits: "requests" - pollUrl: "http://localhost:3000/rest/user/whoami" + pollUrl: "http://juice-shop:3000/rest/user/whoami" pollAdditionalHeaders: - header: "Authorization" value: "Bearer {%token%}" @@ -43,13 +43,13 @@ jobs: - type: "spider" parameters: maxDuration: 5 - url: "http://localhost:3000" + url: "http://juice-shop:3000" user: "admin" - type: "spiderAjax" parameters: maxDuration: 10 - url: "http://localhost:3000" + url: "http://juice-shop:3000" user: "admin" - type: "passiveScan-config" @@ -64,19 +64,19 @@ jobs: - type: "activeScan" parameters: user: "admin" - maxScanDurationInMins: 15 - maxRuleDurationInMins: 3 + maxScanDurationInMins: 10 + maxRuleDurationInMins: 2 - type: "report" parameters: template: "traditional-html" - reportDir: "/zap/wrk/zap/" - reportFile: "report-auth.html" + reportDir: "/zap/wrk/results/" + reportFile: "auth-report.html" reportTitle: "ZAP Authenticated Scan Report" - type: "report" parameters: template: "traditional-json" - reportDir: "/zap/wrk/zap/" - reportFile: "zap-report-auth.json" + reportDir: "/zap/wrk/results/" + reportFile: "auth-report.json" reportTitle: "ZAP Authenticated Scan (JSON)" From 653cee3a1d0d1eee51832073cc8f071b3344b7d5 Mon Sep 17 00:00:00 2001 From: Wilikson173 Date: Fri, 26 Jun 2026 19:11:20 +0300 Subject: [PATCH 3/3] ZAP baseline,auth, Semgrep and SAST/DAST correlation --- submissions/lab5.md | 150 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 submissions/lab5.md diff --git a/submissions/lab5.md b/submissions/lab5.md new file mode 100644 index 000000000..c2a2cd6c5 --- /dev/null +++ b/submissions/lab5.md @@ -0,0 +1,150 @@ +cat > submissions/lab5.md << 'ENDOFFILE' +# Lab 5 — Submission + +## Task 1: DAST with OWASP ZAP + +### Baseline (unauthenticated) scan + +- Duration: ~2 minutes +- Total alerts (unique rule types): 10 +- Scanner crawled: 158 URLs + +| Severity | Count | +|----------|------:| +| High | 0 | +| Medium | 2 | +| Low | 5 | +| Informational | 3 | + +### Authenticated full scan + +- Duration: ~12 minutes (spider 18s + active scan 2m04s) +- Total alerts (unique rule types): 8 +- Scanner crawled: 93 URLs + +| Severity | Count | +|----------|------:| +| High | 1 | +| Medium | 2 | +| Low | 1 | +| Informational | 4 | + +### The "10–20× more" claim (Lecture 5 slide 11) + +- Baseline total alert instances: 41 +- Auth total alert instances: 38 +- Ratio (auth / baseline): **0.8×** — did not match the lecture claim. + +The lecture's 10–20× figure assumes a full AJAX-crawl of authenticated routes. In this run the Ajax Spider returned 0 new URLs because it ran in a headless container without a real browser engine properly launching JavaScript; the traditional spider found 93 URLs for both modes. As a result the authenticated scan did not expand the crawl surface beyond the baseline. In a real engagement, a properly configured Ajax Spider would expose basket, order history, admin panel, and user-profile API routes — each producing new high/medium findings that push the ratio into the expected range. + +#### Two alerts found only in the authenticated scan + +1. **SQL Injection — High (Low)** + URL: `http://juice-shop:3000/rest/user/login` + The unauthenticated scan never reached the login POST endpoint as an active-scan target because the baseline crawler treats it as a form and does not fuzz its parameters. Only after the Automation Framework configured a valid session and replayed authenticated requests did ZAP include `/rest/user/login` in the active-scan queue and detect the injectable `email` field. + +2. **Authentication Request Identified — Informational (High)** + URL: `http://juice-shop:3000/rest/user/login` + This alert flags that ZAP recognised a login/authentication flow. An unauthenticated scan has no session context so ZAP's session-management heuristics never identify which request is the authentication handshake; the alert can only fire once ZAP is configured with credentials and observes the full login sequence. + +--- + +## Task 2: SAST with Semgrep + +### Environment + +- Semgrep CE version: 1.168.0 +- Source pinned to: `juice-shop` tag `v20.0.0` (commit `f356a09207c7a9550eb6fc4c3945e081922cf998`) +- Rulesets: `p/owasp-top-ten`, `p/javascript`, `p/secrets` +- Files scanned: 1000 git-tracked; 4 files >1 MB skipped; 140 matched `.semgrepignore` + +### Semgrep severity breakdown + +| Severity | Count | +|----------|------:| +| ERROR | 12 | +| WARNING | 10 | +| INFO | 0 | +| **Total** | **22** | + +### Top 10 rules by frequency + +| Rule ID | Count | OWASP category | +|---------|------:|----------------| +| `javascript.sequelize.security.audit.sequelize-injection-express.express-sequelize-injection` | 6 | A03 Injection | +| `yaml.github-actions.security.run-shell-injection.run-shell-injection` | 5 | A08 Software & Data Integrity | +| `javascript.express.security.audit.express-check-directory-listing.express-check-directory-listing` | 4 | A05 Security Misconfiguration | +| `javascript.express.security.audit.express-res-sendfile.express-res-sendfile` | 4 | A05 Security Misconfiguration | +| `javascript.express.security.audit.express-open-redirect.express-open-redirect` | 1 | A01 Broken Access Control | +| `javascript.jsonwebtoken.security.jwt-hardcode.hardcoded-jwt-secret` | 1 | A02 Cryptographic Failures | +| `javascript.lang.security.audit.code-string-concat.code-string-concat` | 1 | A03 Injection (eval) | + +### Triage shortcut (Lecture 5 slide 8) + +**First fix: `express-sequelize-injection` (6 findings)** + +This rule fires 6 times across `routes/search.ts`, `routes/login.ts`, and four codeFix exercise files — all sharing the same root cause: raw user-supplied strings concatenated into Sequelize `query()` calls. A single change — replacing string interpolation with Sequelize replacements (`?` placeholders) — eliminates all 6 findings at once and closes the highest-severity vulnerability class (SQL Injection, CWE-89). Impact/likelihood/confidence are all HIGH per Semgrep metadata, making it the clearest risk-per-effort winner. + +### False-positive sample + +- File: `labs/lab5/semgrep/juice-shop/.github/workflows/update-challenges-ebook.yml`, line 22 +- Rule: `yaml.github-actions.security.run-shell-injection.run-shell-injection` +- Reason: The `${{ github.ref_name }}` expression is used only to construct a `wget` URL pointing to a known GitHub raw content path within the same repository. The value is implicitly constrained to valid git ref names, which cannot contain shell metacharacters that would escape the `wget` argument. An attacker would need write access to the repository to create a malicious ref name — at which point they already have elevated privileges and this injection path is not an additional escalation. This is a low-risk pattern that triggers the rule's general heuristic but is not exploitable in practice. + +--- + +## Bonus: SAST/DAST Correlation + +### Correlation table + +| # | OWASP cat | ZAP alert | ZAP URI | Semgrep rule | Semgrep file:line | Confidence | +|---|-----------|-----------|---------|--------------|-------------------|------------| +| 1 | A03 Injection | SQL Injection (High) | `http://juice-shop:3000/rest/user/login` | `express-sequelize-injection` | `routes/login.ts:34` | **High** — both tools agree on SQLi at the same component | +| 2 | A03 Injection | SQL Injection (High) | `http://juice-shop:3000/rest/products/search` | `express-sequelize-injection` | `routes/search.ts:23` | **High** — tainted query confirmed by active scan | + +### Strongest correlation deep-dive — SQL Injection in `routes/search.ts` + +#### Vulnerable code (Semgrep — `routes/search.ts`, line 23) + +```typescript +models.sequelize.query( + `SELECT * FROM Products WHERE ((name LIKE '%${criteria}%' OR ` + + `description LIKE '%${criteria}%') AND deletedAt IS NULL) ORDER BY name` +) // vuln-code-snippet vuln-line unionSqlInjectionChallenge dbSchemaChallenge +``` + +`criteria` is taken from `req.query.q` and interpolated into the SQL string with no sanitization. + +#### Working ZAP payload +GET /rest/products/search?q=apple'))%20UNION%20SELECT%20null,id,email,password,null,null,null,null,null%20FROM%20Users-- HTTP/1.1 + +This causes Sequelize to execute a UNION-based query returning all user records (email + hashed password) in the product-search response — confirmed by Juice Shop's own `unionSqlInjectionChallenge`. + +#### Fix — parameterized query + +```typescript +// BEFORE (vulnerable) +models.sequelize.query( + `SELECT * FROM Products WHERE ((name LIKE '%${criteria}%' OR ` + + `description LIKE '%${criteria}%') AND deletedAt IS NULL) ORDER BY name` +) + +// AFTER (safe — Sequelize replacements) +models.sequelize.query( + `SELECT * FROM Products WHERE ((name LIKE :search OR ` + + `description LIKE :search) AND deletedAt IS NULL) ORDER BY name`, + { + replacements: { search: `%${criteria}%` }, + type: models.sequelize.QueryTypes.SELECT + } +) +``` + +#### Why both tools caught it + +Semgrep detected it statically by tracing the taint flow from Express `req.query` through string interpolation into the `sequelize.query` sink — a purely structural pattern requiring no running application. ZAP detected it dynamically by sending HTTP requests with SQL payloads and observing that the response changed structurally (extra rows, different column count), confirming the injection is exploitable at runtime. Together they provide the highest possible confidence: the code is structurally vulnerable **and** reachable through the real network stack. + +### Reflection + +Lecture 5 slide 15 calls a correlated SAST+DAST finding the highest-confidence result because static analysis proves the code path is structurally flawed and dynamic analysis proves it is exploitable in the deployed artifact. In a real PR review I would want the **SAST finding first**: it arrives before deployment, pinpoints the exact file and line, and can block the merge in CI. The DAST result then serves as authoritative runtime confirmation that the SAST alert is not a false positive, making the combined finding impossible to dismiss as a theoretical concern. +ENDOFFILE