From b4719a99165fae255a8844aa42d56fd6f6925b63 Mon Sep 17 00:00:00 2001 From: cesnimda Date: Sat, 11 Apr 2026 14:30:32 +0200 Subject: [PATCH] Add adversarial security assessment findings --- .gsd/REQUIREMENTS.md | 26 +- .../M013-adversarial-security-assessment.md | 272 ++++++++++++++++++ 2 files changed, 296 insertions(+), 2 deletions(-) create mode 100644 docs/security-assessments/M013-adversarial-security-assessment.md diff --git a/.gsd/REQUIREMENTS.md b/.gsd/REQUIREMENTS.md index c3ba9ea..eef527b 100644 --- a/.gsd/REQUIREMENTS.md +++ b/.gsd/REQUIREMENTS.md @@ -26,6 +26,26 @@ This file is the explicit capability and coverage contract for the project. - Validation: mapped - Notes: Shared/team workflows are not the current product target. +### R018 — Run an adversarial security assessment against the application across input validation, authentication, authorization, API exposure, file uploads, and data exposure. +- Class: operational +- Status: active +- Description: Run an adversarial security assessment against the application across input validation, authentication, authorization, API exposure, file uploads, and data exposure. +- Why it matters: The next milestone is explicitly a hostile security-testing pass intended to find vulnerabilities before attackers do. +- Source: user-security-milestone +- Primary owning slice: M013 +- Validation: Produce verified findings or an explicit no-finding result for each requested attack category. +- Notes: Assessment should assume weak protections and behave like an aggressive tester, not a happy-path reviewer. + +### R019 — For each security issue found, record the vulnerability description, an example exploit input, risk level, and a clear remediation recommendation. +- Class: functional +- Status: active +- Description: For each security issue found, record the vulnerability description, an example exploit input, risk level, and a clear remediation recommendation. +- Why it matters: Security testing is only useful if the output is actionable for remediation and triage. +- Source: user-security-milestone +- Primary owning slice: M013 +- Validation: Each finding includes description, exploit example, risk rating, and fix guidance. +- Notes: If no issue is found in a category, the milestone should still document what was tested and the observed boundary. + ## Validated ### R001 — The user finds a job outside the app, imports it into the app, and starts the application workflow from that imported role. @@ -218,10 +238,12 @@ This file is the explicit capability and coverage contract for the project. | R015 | anti-feature | out-of-scope | none | none | n/a | | R016 | out-of-scope | out-of-scope | none | none | n/a | | R017 | out-of-scope | out-of-scope | none | none | n/a | +| R018 | operational | active | M013 | none | Produce verified findings or an explicit no-finding result for each requested attack category. | +| R019 | functional | active | M013 | none | Each finding includes description, exploit example, risk rating, and fix guidance. | ## Coverage Summary -- Active requirements: 2 -- Mapped to slices: 2 +- Active requirements: 4 +- Mapped to slices: 4 - Validated: 8 (R001, R002, R003, R004, R005, R006, R007, R010) - Unmapped active requirements: 0 diff --git a/docs/security-assessments/M013-adversarial-security-assessment.md b/docs/security-assessments/M013-adversarial-security-assessment.md new file mode 100644 index 0000000..646d889 --- /dev/null +++ b/docs/security-assessments/M013-adversarial-security-assessment.md @@ -0,0 +1,272 @@ +# M013 Adversarial Security Assessment + +## Scope + +Tested as requested: + +- Input validation issues +- Authentication flaws +- Authorization issues +- API security +- File upload vulnerabilities +- Data exposure + +Assessment style: hostile, exploit-oriented, evidence-first. + +## Confirmed Findings + +### 1. Authenticated SSRF via hostname-based loopback bypass in job import preview + +- **Category:** API security / input validation +- **Component:** `JobTrackerApi/Services/JobImport/JobImportService.cs` +- **Endpoint:** `POST /api/jobimport/preview` +- **Risk:** **High** + +#### Vulnerability + +`JobImportService.TryValidateUrl(...)` blocks literal loopback and private IPs, but it does **not** resolve hostnames before allowing the request. That means a hostname that resolves to a loopback/private address can bypass the protection. + +The validator rejects: + +- `http://127.0.0.1:5202/...` +- `http://[::1]:5202/...` +- `http://2130706433:5202/...` + +But it accepted hostnames resolving to loopback, including: + +- `http://127.0.0.1.nip.io:5202/api/auth/config` +- `http://localhost.localdomain:5202/api/auth/config` + +#### Example exploit input + +```http +POST /api/jobimport/preview +Authorization: Bearer +Content-Type: application/json + +{ + "url": "http://127.0.0.1.nip.io:5202/api/auth/config" +} +``` + +Observed result: + +- request was **not** rejected as local/private +- server fetched the internal endpoint +- response progressed to parser failure (`No JobPosting schema found`), which is enough to prove the internal fetch happened + +#### Why this matters + +An authenticated attacker can use the server as an HTTP client against internal-only services or private network resources reachable from the API host. Depending on deployment, this can expose: + +- internal admin/debug endpoints +- cloud metadata services +- internal service meshes +- localhost-only ports +- network topology and response behavior + +#### Clear fix + +- Resolve DNS before allowing the request. +- Reject any hostname whose resolved addresses are loopback, link-local, RFC1918 private, or otherwise internal. +- Re-resolve after redirects, or disable redirects entirely. +- Consider an allowlist of supported job domains instead of general outbound fetching. +- Log and rate-limit preview fetch attempts. + +--- + +### 2. Subjectless signed local JWTs authenticate successfully and can disable owner scoping + +- **Category:** Authentication flaws / authorization issues +- **Components:** + - `JobTrackerApi/Program.cs` + - `Data/JobTrackerContext.cs` + - several owner-scoped controllers relying on EF query filters +- **Risk:** **High** + +#### Vulnerability + +Local JWT validation accepts a correctly signed token **without** a required subject / nameidentifier claim. + +Runtime proof: + +- a signed local JWT with **no** `ClaimTypes.NameIdentifier` / `sub` +- but with valid issuer/audience/signature +- was accepted by the API +- `GET /api/auth/me` returned `200` + +Observed response shape: + +- provider: `external` +- id: `null` +- email echoed from token + +At the same time, owner scoping in `Data/JobTrackerContext.cs` is defined like this: + +```csharp +.HasQueryFilter(x => CurrentUserId == null || x.OwnerUserId == CurrentUserId) +``` + +If `CurrentUserId` is null, the filter collapses to **allow all rows** for owner-scoped entities. + +That is a dangerous composition: + +1. token is authenticated +2. current user id is null +3. owner filters disable themselves +4. endpoints that rely on implicit owner filtering become potentially cross-tenant + +#### Example exploit input + +A signed HS256 JWT using the app signing key, but **omitting** the nameidentifier claim. + +Payload example: + +```json +{ + "iss": "JobTrackerApi", + "aud": "job-tracker-ui", + "nbf": 1775910345, + "exp": 1775913945, + "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress": "ghost@example.com", + "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name": "ghost@example.com" +} +``` + +Observed runtime request: + +```http +GET /api/auth/me +Authorization: Bearer +``` + +Observed result: + +- `200 OK` +- request treated as authenticated even though no user identity key existed for owner scoping + +#### Why this matters + +If an attacker can forge or obtain a valid local signing key, this flaw is not just “login bypass” — it can become a **tenant-boundary bypass** because owner filters stop applying. + +This is especially serious in environments where: + +- the dev signing key is reused +- a staging or preview environment leaks the local JWT key +- operational mistakes deploy development auth settings + +#### Clear fix + +- Require a non-empty subject / nameidentifier claim during local JWT validation. +- Reject authenticated requests whose token does not map to a concrete application user identity. +- Change query filters so `CurrentUserId == null` means **deny**, not allow-all, for owner-scoped entities. +- Avoid relying on implicit global filters alone for sensitive raw-id endpoints; add explicit owner predicates in controller queries. + +Example direction: + +- validate a required identity claim in the JWT bearer events pipeline +- make owner filter logic equivalent to `CurrentUserId != null && x.OwnerUserId == CurrentUserId` + +--- + +## High-Risk Candidates Not Fully Confirmed In This Runtime + +These are not counted as confirmed findings yet, but they remain serious candidates. + +### A. Raw-id child endpoints rely on implicit owner scoping + +- **Category:** Authorization issues +- **Components:** + - `JobTrackerApi/Controllers/AttachmentsController.cs` + - `JobTrackerApi/Controllers/CorrespondenceController.cs` + - selected job-linked endpoints +- **Risk:** **Medium to High** + +Patterns observed: + +- existence checks like `AnyAsync(j => j.Id == jobId)` +- later child fetches through parent relationships +- reliance on EF global filters instead of explicit per-request owner predicates + +If the owner filter is ever bypassed, weakened, or accidentally ignored, these become cross-user read/write primitives. + +This runtime could not prove the full cross-user exploit path because the active SQLite file is missing core domain tables (`Companies`, `JobApplications`, `RuleSettings`) and those requests fail before authorization behavior can be fully exercised. + +#### Suggested fix + +Add explicit owner predicates in the endpoint queries themselves instead of trusting global filters as the only boundary. + +--- + +## Tested Surfaces With No Confirmed Finding In This Pass + +### Anonymous API reachability + +Observed anonymous results in this runtime: + +- `GET /api/export/jobs` → `401` +- `POST /api/backup/encrypted` → `401` +- `POST /api/jobimport/preview` → `401` +- `POST /api/client-errors` → `401` + +This is better than the earlier pre-hardening posture. + +### Gmail OAuth callback + +- `GET /api/gmail/oauth/callback?code=fake&state=fake` returned a generic failure page +- no secret data exposure observed in this pass + +### File upload path traversal via visible filename + +Code review on: + +- `AuthController.UploadAvatar` +- `AttachmentsController.Upload` +- `AttachmentsController.Rename` +- `ProfileCvController.Upload` + +Current handling uses `Path.GetFileName(...)` and generated storage names, which is a reasonable defense against straightforward path traversal through user-supplied filenames. + +No confirmed traversal exploit in this pass. + +## Recommended Remediation Order + +1. **Fix authenticated SSRF in job import preview** + - hostname resolution checks + - no internal/private destinations + - preferably domain allowlist + +2. **Fix JWT subjectless-auth acceptance and owner-filter allow-all behavior** + - require subject/nameidentifier + - reject tokens that do not map to a real app identity + - change owner filters to deny on null current user + +3. **Harden raw-id owner-sensitive endpoints with explicit owner predicates** + - attachments + - correspondence + - job-linked child endpoints + +4. **Run a second exploit pass after the fixes** + - repeat cross-user probes + - retest SSRF bypasses + - fuzz upload/parser surfaces further + +## Evidence Summary + +### Runtime probes performed + +- anonymous reachability checks against auth/config, csrf, auth/me, client-errors, jobimport preview, export, backup, and Gmail callback +- authenticated SSRF probes against job import preview using loopback-resolving hostnames +- authenticated malformed-token probe using a signed local JWT without subject/nameidentifier + +### Key observed outputs + +- `/api/jobimport/preview` accepted `http://127.0.0.1.nip.io:5202/api/auth/config` +- `/api/auth/me` returned `200` for a signed local JWT without subject/nameidentifier +- owner filters in `JobTrackerContext` explicitly allow all rows when `CurrentUserId == null` + +## Honest Boundaries + +- Some cross-user raw-id probes were limited by the local runtime using an incomplete SQLite schema. +- Those areas are reported as **high-risk candidates**, not falsely upgraded to confirmed findings. +- The two confirmed findings above are supported by direct runtime evidence plus code-path verification.