Add adversarial security assessment findings
This commit is contained in:
+24
-2
@@ -26,6 +26,26 @@ This file is the explicit capability and coverage contract for the project.
|
|||||||
- Validation: mapped
|
- Validation: mapped
|
||||||
- Notes: Shared/team workflows are not the current product target.
|
- 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
|
## Validated
|
||||||
|
|
||||||
### R001 — The user finds a job outside the app, imports it into the app, and starts the application workflow from that imported role.
|
### 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 |
|
| R015 | anti-feature | out-of-scope | none | none | n/a |
|
||||||
| R016 | out-of-scope | 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 |
|
| 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
|
## Coverage Summary
|
||||||
|
|
||||||
- Active requirements: 2
|
- Active requirements: 4
|
||||||
- Mapped to slices: 2
|
- Mapped to slices: 4
|
||||||
- Validated: 8 (R001, R002, R003, R004, R005, R006, R007, R010)
|
- Validated: 8 (R001, R002, R003, R004, R005, R006, R007, R010)
|
||||||
- Unmapped active requirements: 0
|
- Unmapped active requirements: 0
|
||||||
|
|||||||
@@ -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 <valid local token>
|
||||||
|
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 <signed token without subject>
|
||||||
|
```
|
||||||
|
|
||||||
|
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.
|
||||||
Reference in New Issue
Block a user