feat(lab5): ZAP baseline + auth + Semgrep + correlation + fixed scripts

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:
 <details>
 <summary>⚠️ Common Pitfalls</summary>
 
-- 🚨 **`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.

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', ''))

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,16 +15,16 @@ 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"
           loggedInRegex: ".*\"user\".*"
           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)"

submissions/lab5.md

@@ -0,0 +1,110 @@
+# Lab 5 — Submission
+
+## Task 1: DAST with OWASP ZAP
+
+### Baseline (unauthenticated) scan
+- Duration: 1.5 minutes
+- Total alerts: 10
+
+| Severity | Count |
+|----------|------:|
+| High | 0 |
+| Medium | 2 |
+| Low | 5 |
+| Informational | 3 |
+
+### Authenticated full scan
+- Duration: 4.5 minutes
+- Total alerts: 12
+
+| Severity | Count |
+|----------|------:|
+| High | 1 |
+| Medium | 4 |
+| Low | 3 |
+| Informational | 4 |
+
+### The "10–20× more" claim (Lecture 5 slide 11)
+- Ratio (auth alerts / baseline alerts): 1.2×
+- Did your run match the lecture's ratio? 
+  No, my run showed a ratio of 1.2× for unique alert *types*. The lecture's claim likely refers to the sheer volume of vulnerable *instances* (URLs/parameters), as an authenticated crawler accesses exponentially more pages behind the login. Our script only counted unique risk categories.
+- Pick **two specific alerts** that only the authenticated scan found. For each:
+  1. **SQL Injection (High)**
+     - Why was it unreachable to the unauthenticated scan? The vulnerable endpoint or parameter is located behind the login barrier, so the baseline crawler couldn't reach the page to inject the payload.
+  2. **Session ID in URL Rewrite (Medium)**
+     - Why was it unreachable to the unauthenticated scan? This vulnerability relies on a user session being actively established; since the baseline scan does not log in, it cannot trigger or observe session management flaws.
+
+
+
+
+
+## Task 2: SAST with Semgrep
+
+### 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 |
+| `yaml.github-actions.security.run-shell-injection.run-shell-injection` | 5 | A03 |
+| `javascript.express.security.audit.express-res-sendfile.express-res-sendfile` | 4 | A01 |
+| `javascript.express.security.audit.express-check-directory-listing.express-check-directory-listing` | 4 | A01 |
+| `javascript.jsonwebtoken.security.jwt-hardcode.hardcoded-jwt-secret` | 1 | A02 |
+| `javascript.express.security.audit.express-open-redirect.express-open-redirect` | 1 | A01 |
+| `javascript.lang.security.audit.code-string-concat.code-string-concat` | 1 | A03 |
+
+### Triage shortcut (Lecture 5 slide 8)
+Looking at the top 10 — which **one rule** would you fix first if you had time for only one?
+I would fix `express-sequelize-injection` first. It has the highest frequency among the vulnerabilities found and represents a severe flaw (SQL Injection - A03), meaning fixing the parameter binding at the ORM level here would quickly eliminate a large cluster of critical vulnerabilities.
+
+### False-positive sample
+File path: `data/static/codefixes/unionSqlInjectionChallenge_1.ts`
+Rule: `express-sequelize-injection`
+Reason: This file is just a static fixture containing code snippets used to display code fixes in the UI; it's not actually executed by the backend, making this finding a false positive.
+
+
+
+
+
+
+  ## Bonus: SAST/DAST Correlation
+
+### Correlation table
+| # | OWASP cat | ZAP alert | ZAP URI | Semgrep rule | Semgrep file:line | Confidence |
+|---|-----------|-----------|---------|--------------|-------------------|------------|
+| 1 | A03 Injection | SQL Injection | `/rest/products/search?q=...` | `express-sequelize-injection` | `routes/search.ts:23` | High (both agree) |
+| 2 | A03 Injection | SQL Injection | `/rest/user/login` (email) | `express-sequelize-injection` | `routes/login.ts:34` | High (both agree) |
+
+### Strongest correlation deep-dive
+**1. Vulnerable code (from `routes/search.ts:23`)**
+```typescript
+models.sequelize.query(`SELECT * FROM Products WHERE ((name LIKE '%${criteria}%' OR description LIKE '%${criteria}%') AND deletedAt IS NULL) ORDER BY name`)
+```
+
+**2. Working payload (from ZAP report)**
+```
+URL: http://juice-shop:3000/rest/products/search?q=%27%28
+Parameter: q
+Attack: '(
+```
+
+**3. The fix**
+Use parameterized queries (bind variables) instead of string interpolation:
+```typescript
+models.sequelize.query(
+  'SELECT * FROM Products WHERE ((name LIKE :criteria OR description LIKE :criteria) AND deletedAt IS NULL) ORDER BY name',
+  { replacements: { criteria: `%${criteria}%` } }
+)
+```
+
+**4. Why both tools caught it**
+Semgrep caught it because the code directly interpolates a user-controlled variable (`criteria` from `req.query.q`) into a raw SQL query string inside the `sequelize.query` call—a classic static pattern. ZAP caught it because during its active crawl it injected SQL metacharacters like `'(` into the `q` parameter and observed a database syntax error or unexpected behavior in the response.
+
+### Reflection (2-3 sentences)
+Lecture 5 slide 15 calls this "the highest-confidence finding type." In a real PR review, I would want the **SAST finding** first. SAST points exactly to the file and line number (`routes/search.ts:23`), which makes it trivial for a developer to implement a fix immediately. The DAST evidence is then incredibly valuable to prove the exploitability of the finding and to verify that the fix actually resolved the issue at runtime.