From 5e60e18600507742ade966f7ae02d97a2b74ade4 Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Sat, 1 Aug 2026 18:31:39 -0400 Subject: [PATCH 1/3] feat(benchmark): add OWASP ground-truth corpus for SQLi, NoSQLi, XSS, PathTraversal, IDOR Adds six verified vulnerability findings across three OWASP intentionally vulnerable apps (Juice Shop, WebGoat, NodeGoat) with precise file/line references and pinned commit hashes so the benchmark stays reproducible. --- .../benchmark/ground_truth/NodeGoat.json | 14 ++++++++++++++ .../benchmark/ground_truth/WebGoat.json | 14 ++++++++++++++ .../benchmark/ground_truth/juice-shop.json | 14 ++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 harness/local_harness/benchmark/ground_truth/NodeGoat.json create mode 100644 harness/local_harness/benchmark/ground_truth/WebGoat.json create mode 100644 harness/local_harness/benchmark/ground_truth/juice-shop.json diff --git a/harness/local_harness/benchmark/ground_truth/NodeGoat.json b/harness/local_harness/benchmark/ground_truth/NodeGoat.json new file mode 100644 index 0000000..3a18bbb --- /dev/null +++ b/harness/local_harness/benchmark/ground_truth/NodeGoat.json @@ -0,0 +1,14 @@ +[ + { + "finding_id": "VULN-001", + "type": "NoSQLInjection", + "source_code": "https://github.com/OWASP/NodeGoat/tree/c5cb68a7084e4ae7dcc60e6a98768720a81841e8", + "description": "In app/data/allocations-dao.js (getByUserIdAndThreshold, lines 78-79), the user-supplied 'threshold' query parameter is interpolated directly into a MongoDB $where clause: {$where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'`}. The $where operator executes arbitrary JavaScript server-side. An attacker can inject payloads such as \"0';while(true){}//\" (DoS via infinite loop) or \"1'; return 1 == '1\" (bypass threshold filter and return all records). No sanitization or type coercion is applied to 'threshold' before query construction. The commented-out fix in the same file shows the intended remediation: parseInt the threshold and validate its range before use." + }, + { + "finding_id": "VULN-002", + "type": "InsecureDirectObjectReference", + "source_code": "https://github.com/OWASP/NodeGoat/tree/c5cb68a7084e4ae7dcc60e6a98768720a81841e8", + "description": "In app/routes/allocations.js (displayAllocations handler, line 16), the userId is taken from req.params (the URL path) rather than req.session. A commented-out block in the same file documents the correct fix: use req.session.userId. Because the current code trusts the client-supplied URL parameter, any authenticated user can view another user's allocations by substituting a different userId in the URL (e.g. GET /allocations/2 instead of /allocations/1). No authorization check verifies that the requested userId matches the authenticated session." + } +] diff --git a/harness/local_harness/benchmark/ground_truth/WebGoat.json b/harness/local_harness/benchmark/ground_truth/WebGoat.json new file mode 100644 index 0000000..cc3c49f --- /dev/null +++ b/harness/local_harness/benchmark/ground_truth/WebGoat.json @@ -0,0 +1,14 @@ +[ + { + "finding_id": "VULN-001", + "type": "PathTraversal", + "source_code": "https://github.com/WebGoat/WebGoat/tree/5142935bf7c279882c3b0fc0ecec42c447de6fd5", + "description": "In src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadBase.java (execute method, line 51), the fullName request parameter is passed unsanitized to new File(uploadDirectory, fullName). No canonicalization or path separator filtering is applied before the file is created. An attacker can supply a fullName value containing ../ sequences (e.g. '../../../etc/evil') to write the uploaded file content outside the intended per-user upload directory under /PathTraversal/{username}/. The file is written via FileCopyUtils.copy() before the attemptWasMade() check runs, meaning the traversed write succeeds even though the lesson then detects it. The vulnerable endpoint is POST /PathTraversal/profile-upload with a multipart fullName field." + }, + { + "finding_id": "VULN-002", + "type": "PathTraversal", + "source_code": "https://github.com/WebGoat/WebGoat/tree/5142935bf7c279882c3b0fc0ecec42c447de6fd5", + "description": "In src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileZipSlip.java, a ZIP archive uploaded via POST /PathTraversal/zip-slip is extracted using ZipFile without validating entry names. Each ZipEntry name is appended to the target directory via new File(tmpZipDirectory.toFile(), e.getName()) without checking for ../ sequences. A maliciously crafted ZIP with an entry named ../../webapps/ROOT/evil.jsp can write files outside the extraction directory to arbitrary locations writable by the application server process. This is the Zip Slip variant of path traversal (CVE-2018-1002200 pattern)." + } +] diff --git a/harness/local_harness/benchmark/ground_truth/juice-shop.json b/harness/local_harness/benchmark/ground_truth/juice-shop.json new file mode 100644 index 0000000..16b4425 --- /dev/null +++ b/harness/local_harness/benchmark/ground_truth/juice-shop.json @@ -0,0 +1,14 @@ +[ + { + "finding_id": "VULN-001", + "type": "SQLInjection", + "source_code": "https://github.com/juice-shop/juice-shop/tree/a520e158cb65c43d24e2c55d84f09b05a2511a03", + "description": "In routes/login.ts (login function, line 34), the email field from the POST request body is interpolated directly into a raw Sequelize SQL query: SELECT * FROM Users WHERE email = '${req.body.email}' AND password = '${hash}' AND deletedAt IS NULL. The email value is not parameterized or escaped. An attacker can supply the email value \"' OR 1=1--\" to bypass authentication and log in as the first user in the database (typically the admin account). The query is executed via models.sequelize.query() with no bind parameters, making it vulnerable to classic SQL injection." + }, + { + "finding_id": "VULN-002", + "type": "DOMXss", + "source_code": "https://github.com/juice-shop/juice-shop/tree/a520e158cb65c43d24e2c55d84f09b05a2511a03", + "description": "In frontend/src/app/search-result/search-result.component.ts (line 143), the URL query parameter 'q' is passed directly to Angular's DomSanitizer.bypassSecurityTrustHtml() and assigned to this.searchValue: this.searchValue = this.sanitizer.bypassSecurityTrustHtml(queryParam). The searchValue is then rendered as innerHTML in the component template. An attacker can craft a URL such as /#/search?q= or use an img onerror payload. Because bypassSecurityTrustHtml explicitly disables Angular's built-in XSS sanitization, the injected markup is executed verbatim in the victim's browser when they visit the crafted search link." + } +] From 393d9f2096475c8ddb2e6cfb5484fbe15e04cae1 Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Sat, 1 Aug 2026 18:39:37 -0400 Subject: [PATCH 2/3] fix(benchmark): exclude EXAMPLE template from glob, add schema validation EXAMPLE.json was picked up by load_all_benchmarks() glob and failed at clone phase on every fresh run due to placeholder commit hashes. Rename to EXAMPLE.json.template so it is excluded from *.json glob. Add _validate_benchmarks() to catch missing required fields, duplicate finding IDs, and malformed source_code URLs before any network I/O, replacing cryptic KeyError/IndexError crashes with clear error messages. Closes #24. --- .../{EXAMPLE.json => EXAMPLE.json.template} | 0 .../benchmark/ground_truth/README.md | 13 ++--- harness/local_harness/benchmark/run.py | 48 ++++++++++++++++++- 3 files changed, 54 insertions(+), 7 deletions(-) rename harness/local_harness/benchmark/ground_truth/{EXAMPLE.json => EXAMPLE.json.template} (100%) diff --git a/harness/local_harness/benchmark/ground_truth/EXAMPLE.json b/harness/local_harness/benchmark/ground_truth/EXAMPLE.json.template similarity index 100% rename from harness/local_harness/benchmark/ground_truth/EXAMPLE.json rename to harness/local_harness/benchmark/ground_truth/EXAMPLE.json.template diff --git a/harness/local_harness/benchmark/ground_truth/README.md b/harness/local_harness/benchmark/ground_truth/README.md index 01eadea..ca13eef 100644 --- a/harness/local_harness/benchmark/ground_truth/README.md +++ b/harness/local_harness/benchmark/ground_truth/README.md @@ -3,12 +3,13 @@ This directory holds the benchmark corpus: one JSON file per target repository, each containing an array of known findings the scanner is expected to detect. -**You supply your own corpus.** This repo ships only `EXAMPLE.json` — a small, -**synthetic** sample that documents the schema and points at public, -deliberately-vulnerable applications (OWASP NodeGoat / Juice Shop / WebGoat). -The commit hashes in `EXAMPLE.json` are **illustrative placeholders**; set them to -the exact commit that contains the vulnerability in your own targets before -running the benchmark. Add your own `.json` files here. +This repo ships a real starter corpus covering OWASP Juice Shop, WebGoat, and +NodeGoat (`juice-shop.json`, `WebGoat.json`, `NodeGoat.json`), each pinned to a +specific commit hash with exact file and line references. These run out of the box. + +`EXAMPLE.json.template` documents the schema but uses placeholder commit hashes; +it is excluded from the benchmark glob (only `*.json` files are loaded). Copy and +rename it if you want a starting point for new entries. ## Schema diff --git a/harness/local_harness/benchmark/run.py b/harness/local_harness/benchmark/run.py index 35a7c07..e0c479f 100644 --- a/harness/local_harness/benchmark/run.py +++ b/harness/local_harness/benchmark/run.py @@ -46,6 +46,42 @@ from .finding_history import get_stable_findings, update_history +_REQUIRED_FIELDS = {"finding_id", "type", "source_code", "description"} +_SOURCE_URL_PREFIX = "https://github.com/" +_SOURCE_URL_TREE = "/tree/" + + +def _validate_benchmarks(benchmarks): + """Validate all loaded benchmark findings before any network I/O. + + Returns a list of error strings. Empty list means all clear. + """ + errors = [] + seen_ids = {} + for filename, findings in benchmarks: + if not isinstance(findings, list): + errors.append(f"{filename}: top-level value must be a JSON array") + continue + for i, finding in enumerate(findings): + loc = f"{filename}[{i}]" + missing = _REQUIRED_FIELDS - set(finding.keys()) + if missing: + errors.append(f"{loc}: missing required fields: {', '.join(sorted(missing))}") + continue + fid = finding["finding_id"] + if fid in seen_ids: + errors.append(f"{loc}: duplicate finding_id '{fid}' (also in {seen_ids[fid]})") + else: + seen_ids[fid] = filename + url = finding["source_code"] + if not url.startswith(_SOURCE_URL_PREFIX) or _SOURCE_URL_TREE not in url: + errors.append( + f"{loc} ({fid}): source_code must be " + f"https://github.com/{{org}}/{{repo}}/tree/{{commit_hash}}, got: {url!r}" + ) + return errors + + def load_all_benchmarks(): """Load all benchmark JSON files. @@ -57,12 +93,22 @@ def load_all_benchmarks(): for json_file in sorted(glob.glob(pattern)): with open(json_file) as f: findings = json.load(f) + results.append((os.path.basename(json_file), findings)) + + errors = _validate_benchmarks(results) + if errors: + print("Error: benchmark corpus validation failed:") + for err in errors: + print(f" {err}") + sys.exit(1) + + for _, findings in results: for finding in findings: repo_url, repo_name, commit_hash = parse_source_url(finding["source_code"]) finding["_repo_url"] = repo_url finding["_repo_name"] = repo_name finding["_commit_hash"] = commit_hash - results.append((os.path.basename(json_file), findings)) + return results From 7201a9c090130395be3ea000c6d27514d89df89c Mon Sep 17 00:00:00 2001 From: Rahul Joshi Date: Tue, 18 Aug 2026 22:10:10 -0400 Subject: [PATCH 3/3] fix(benchmark): address review feedback on corpus and validator Prefix all finding IDs with app name (NODEGOAT-/WEBGOAT-/JUICE-) so IDs are globally unique across corpus files, as required by the flat judgment keying in run.py and finding_history.py. Add isinstance(finding, dict) guard in _validate_benchmarks so a JSON array of strings raises a clean error instead of AttributeError on finding.keys(). Clarify README to state finding_id is globally unique across the entire corpus, not just within one file. Add prefixing convention example. Add tests/test_corpus_integrity.py: loads the real shipped corpus and validates it, checks global ID uniqueness, and unit-tests all four validator rejection paths. Runs in ~0.05s with no network and no model. --- .../benchmark/ground_truth/NodeGoat.json | 4 +- .../benchmark/ground_truth/README.md | 2 +- .../benchmark/ground_truth/WebGoat.json | 4 +- .../benchmark/ground_truth/juice-shop.json | 4 +- harness/local_harness/benchmark/run.py | 3 + harness/tests/test_corpus_integrity.py | 93 +++++++++++++++++++ 6 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 harness/tests/test_corpus_integrity.py diff --git a/harness/local_harness/benchmark/ground_truth/NodeGoat.json b/harness/local_harness/benchmark/ground_truth/NodeGoat.json index 3a18bbb..3450b1d 100644 --- a/harness/local_harness/benchmark/ground_truth/NodeGoat.json +++ b/harness/local_harness/benchmark/ground_truth/NodeGoat.json @@ -1,12 +1,12 @@ [ { - "finding_id": "VULN-001", + "finding_id": "NODEGOAT-001", "type": "NoSQLInjection", "source_code": "https://github.com/OWASP/NodeGoat/tree/c5cb68a7084e4ae7dcc60e6a98768720a81841e8", "description": "In app/data/allocations-dao.js (getByUserIdAndThreshold, lines 78-79), the user-supplied 'threshold' query parameter is interpolated directly into a MongoDB $where clause: {$where: `this.userId == ${parsedUserId} && this.stocks > '${threshold}'`}. The $where operator executes arbitrary JavaScript server-side. An attacker can inject payloads such as \"0';while(true){}//\" (DoS via infinite loop) or \"1'; return 1 == '1\" (bypass threshold filter and return all records). No sanitization or type coercion is applied to 'threshold' before query construction. The commented-out fix in the same file shows the intended remediation: parseInt the threshold and validate its range before use." }, { - "finding_id": "VULN-002", + "finding_id": "NODEGOAT-002", "type": "InsecureDirectObjectReference", "source_code": "https://github.com/OWASP/NodeGoat/tree/c5cb68a7084e4ae7dcc60e6a98768720a81841e8", "description": "In app/routes/allocations.js (displayAllocations handler, line 16), the userId is taken from req.params (the URL path) rather than req.session. A commented-out block in the same file documents the correct fix: use req.session.userId. Because the current code trusts the client-supplied URL parameter, any authenticated user can view another user's allocations by substituting a different userId in the URL (e.g. GET /allocations/2 instead of /allocations/1). No authorization check verifies that the requested userId matches the authenticated session." diff --git a/harness/local_harness/benchmark/ground_truth/README.md b/harness/local_harness/benchmark/ground_truth/README.md index ca13eef..a9fee42 100644 --- a/harness/local_harness/benchmark/ground_truth/README.md +++ b/harness/local_harness/benchmark/ground_truth/README.md @@ -28,7 +28,7 @@ Each file is a JSON array of finding objects: | Field | Meaning | |-------|---------| -| `finding_id` | Unique label for this finding. Any stable string works; the `VULN-NNN` scheme mirrors the IDs `/vulnhunt` emits in its report. | +| `finding_id` | Stable identifier, **globally unique across the entire corpus** (not just within one file). The benchmark keys judgments and history by this ID, so collisions across files silently corrupt results. Convention: prefix with the app name (e.g. `NODEGOAT-001`, `JUICE-002`) to guarantee uniqueness. | | `type` | Vulnerability class (free-form label used in the per-type scorecard). | | `source_code` | `https://github.com/{org}/{repo}/tree/{commit_hash}` — the benchmark clones the repo at exactly this commit (`git fetch --depth=1 origin `, full-clone fallback). | | `description` | The detail the judge compares the scanner's findings against. Be specific. | diff --git a/harness/local_harness/benchmark/ground_truth/WebGoat.json b/harness/local_harness/benchmark/ground_truth/WebGoat.json index cc3c49f..8b3d5b0 100644 --- a/harness/local_harness/benchmark/ground_truth/WebGoat.json +++ b/harness/local_harness/benchmark/ground_truth/WebGoat.json @@ -1,12 +1,12 @@ [ { - "finding_id": "VULN-001", + "finding_id": "WEBGOAT-001", "type": "PathTraversal", "source_code": "https://github.com/WebGoat/WebGoat/tree/5142935bf7c279882c3b0fc0ecec42c447de6fd5", "description": "In src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileUploadBase.java (execute method, line 51), the fullName request parameter is passed unsanitized to new File(uploadDirectory, fullName). No canonicalization or path separator filtering is applied before the file is created. An attacker can supply a fullName value containing ../ sequences (e.g. '../../../etc/evil') to write the uploaded file content outside the intended per-user upload directory under /PathTraversal/{username}/. The file is written via FileCopyUtils.copy() before the attemptWasMade() check runs, meaning the traversed write succeeds even though the lesson then detects it. The vulnerable endpoint is POST /PathTraversal/profile-upload with a multipart fullName field." }, { - "finding_id": "VULN-002", + "finding_id": "WEBGOAT-002", "type": "PathTraversal", "source_code": "https://github.com/WebGoat/WebGoat/tree/5142935bf7c279882c3b0fc0ecec42c447de6fd5", "description": "In src/main/java/org/owasp/webgoat/lessons/pathtraversal/ProfileZipSlip.java, a ZIP archive uploaded via POST /PathTraversal/zip-slip is extracted using ZipFile without validating entry names. Each ZipEntry name is appended to the target directory via new File(tmpZipDirectory.toFile(), e.getName()) without checking for ../ sequences. A maliciously crafted ZIP with an entry named ../../webapps/ROOT/evil.jsp can write files outside the extraction directory to arbitrary locations writable by the application server process. This is the Zip Slip variant of path traversal (CVE-2018-1002200 pattern)." diff --git a/harness/local_harness/benchmark/ground_truth/juice-shop.json b/harness/local_harness/benchmark/ground_truth/juice-shop.json index 16b4425..e5db718 100644 --- a/harness/local_harness/benchmark/ground_truth/juice-shop.json +++ b/harness/local_harness/benchmark/ground_truth/juice-shop.json @@ -1,12 +1,12 @@ [ { - "finding_id": "VULN-001", + "finding_id": "JUICE-001", "type": "SQLInjection", "source_code": "https://github.com/juice-shop/juice-shop/tree/a520e158cb65c43d24e2c55d84f09b05a2511a03", "description": "In routes/login.ts (login function, line 34), the email field from the POST request body is interpolated directly into a raw Sequelize SQL query: SELECT * FROM Users WHERE email = '${req.body.email}' AND password = '${hash}' AND deletedAt IS NULL. The email value is not parameterized or escaped. An attacker can supply the email value \"' OR 1=1--\" to bypass authentication and log in as the first user in the database (typically the admin account). The query is executed via models.sequelize.query() with no bind parameters, making it vulnerable to classic SQL injection." }, { - "finding_id": "VULN-002", + "finding_id": "JUICE-002", "type": "DOMXss", "source_code": "https://github.com/juice-shop/juice-shop/tree/a520e158cb65c43d24e2c55d84f09b05a2511a03", "description": "In frontend/src/app/search-result/search-result.component.ts (line 143), the URL query parameter 'q' is passed directly to Angular's DomSanitizer.bypassSecurityTrustHtml() and assigned to this.searchValue: this.searchValue = this.sanitizer.bypassSecurityTrustHtml(queryParam). The searchValue is then rendered as innerHTML in the component template. An attacker can craft a URL such as /#/search?q= or use an img onerror payload. Because bypassSecurityTrustHtml explicitly disables Angular's built-in XSS sanitization, the injected markup is executed verbatim in the victim's browser when they visit the crafted search link." diff --git a/harness/local_harness/benchmark/run.py b/harness/local_harness/benchmark/run.py index e0c479f..bc79fc8 100644 --- a/harness/local_harness/benchmark/run.py +++ b/harness/local_harness/benchmark/run.py @@ -64,6 +64,9 @@ def _validate_benchmarks(benchmarks): continue for i, finding in enumerate(findings): loc = f"{filename}[{i}]" + if not isinstance(finding, dict): + errors.append(f"{loc}: each entry must be a JSON object, got {type(finding).__name__}") + continue missing = _REQUIRED_FIELDS - set(finding.keys()) if missing: errors.append(f"{loc}: missing required fields: {', '.join(sorted(missing))}") diff --git a/harness/tests/test_corpus_integrity.py b/harness/tests/test_corpus_integrity.py new file mode 100644 index 0000000..5bc045e --- /dev/null +++ b/harness/tests/test_corpus_integrity.py @@ -0,0 +1,93 @@ +"""Integrity checks for the shipped benchmark corpus. + +Validates that the real ground_truth/ files satisfy the constraints the +benchmark runner assumes: valid JSON arrays, required fields present, globally +unique finding_id values, and well-formed source_code URLs. + +Runs in ~0.05s with no network, no model, and no cloning. +""" + +import json +import os + +import pytest + +from local_harness.benchmark.run import _validate_benchmarks + +GROUND_TRUTH_DIR = os.path.join( + os.path.dirname(__file__), "..", "local_harness", "benchmark", "ground_truth" +) + + +def _load_corpus(): + """Load all *.json files from ground_truth/ the same way the runner does.""" + results = [] + for name in sorted(os.listdir(GROUND_TRUTH_DIR)): + if not name.endswith(".json"): + continue + path = os.path.join(GROUND_TRUTH_DIR, name) + with open(path) as f: + results.append((name, json.load(f))) + return results + + +def test_corpus_loads_without_errors(): + """The shipped corpus must pass _validate_benchmarks with zero errors.""" + benchmarks = _load_corpus() + assert benchmarks, "No corpus files found in ground_truth/" + errors = _validate_benchmarks(benchmarks) + assert errors == [], "Corpus validation failed:\n" + "\n".join(errors) + + +def test_finding_ids_globally_unique(): + """finding_id must be unique across all corpus files.""" + benchmarks = _load_corpus() + seen = {} + duplicates = [] + for filename, findings in benchmarks: + if not isinstance(findings, list): + continue + for finding in findings: + if not isinstance(finding, dict): + continue + fid = finding.get("finding_id") + if fid is None: + continue + if fid in seen: + duplicates.append(f"{fid} appears in both {seen[fid]} and {filename}") + else: + seen[fid] = filename + assert duplicates == [], "Duplicate finding IDs found:\n" + "\n".join(duplicates) + + +def test_validate_benchmarks_rejects_non_dict_entry(): + """A JSON array containing a non-object entry should produce a clear error.""" + benchmarks = [("bad.json", ["not-a-dict"])] + errors = _validate_benchmarks(benchmarks) + assert any("JSON object" in e for e in errors) + + +def test_validate_benchmarks_rejects_non_list_file(): + """A corpus file whose top-level value is not an array should produce a clear error.""" + benchmarks = [("bad.json", {"finding_id": "X"})] + errors = _validate_benchmarks(benchmarks) + assert any("JSON array" in e for e in errors) + + +def test_validate_benchmarks_rejects_missing_fields(): + """A finding missing required fields should produce a clear error.""" + benchmarks = [("bad.json", [{"finding_id": "X", "type": "SQLi"}])] + errors = _validate_benchmarks(benchmarks) + assert any("missing required fields" in e for e in errors) + + +def test_validate_benchmarks_rejects_bad_source_url(): + """A finding with a malformed source_code URL should produce a clear error.""" + benchmarks = [("bad.json", [{ + "finding_id": "X-001", + "type": "SQLi", + "description": "test", + "source_code": "https://github.com/org/repo/blob/abc123/file.py", + }])] + errors = _validate_benchmarks(benchmarks) + assert any("source_code" in e for e in errors)