Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions labs/lab2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion labs/lab2/threagile-model.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
5 changes: 4 additions & 1 deletion labs/lab5.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
12 changes: 4 additions & 8 deletions labs/lab5/scripts/compare_zap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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', ''))
Expand Down
26 changes: 13 additions & 13 deletions labs/lab5/scripts/zap-auth.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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$"
Expand All @@ -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%}"
Expand All @@ -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"
Expand All @@ -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)"
150 changes: 150 additions & 0 deletions submissions/lab5.md
Original file line number Diff line number Diff line change
@@ -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
Loading