fix(diagnostics): a passing check should not report an HTTP status - #152
Conversation
The self-test showed "PASS · TSP+DIDComm mediator accepts this wallet's origin — answered (HTTP 405)". The first person to read it asked what was broken. Nothing was. The status was named on purpose, reasoning that seeing the number would make it obvious a 4xx is expected and not the thing being tested. It did the opposite. A green PASS next to a 405 reads as a contradiction, and the reader has to already understand CORS probing to resolve it — which is precisely the knowledge this panel exists to not require. `checkCorsReachable` sends a plain GET against the same endpoint that would fail, so a POST-only auth route answers 405 and a REST base with no handler at its bare path answers 404. Both are complete passes: ANY status proves the origin was allowed, because a CORS refusal has no status to read at all. The number answers a question nobody asked, in a place where an unexplained number reads as a fault. So `checkCorsReachable` no longer returns it — a field whose only consumer was that misleading line is not worth keeping for a future caller who would be wrong to use it. Failures are untouched: they carry their detail and a stable `code`, which is where the diagnosis belongs. The REST row also gains the "with this extension's origin" clause it was missing, so both rows now say what was actually proven rather than what was received. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
🛡️ AI Agentic Security Code Review3 findings need a human to review/validate. Mandatory to check: 🔒 Security Code Review Report Details🛡️ Security Code Review Report — PR #152
🗺️ Scan CoverageModules scanned: 1 · with findings: 1 · files: 1 · findings: 4
Executive Summary
🔒 Security Issues
|
| Field | Detail |
|---|---|
| Severity | MEDIUM |
| Location | packages/extension/src/offscreen.ts:736 |
| Finding ID | github_pr-a4a1a68d0853 |
| CWE | CWE-346, CWE-942, CWE-290 |
| OWASP | A05:2021 - Security Misconfiguration |
| MITRE ATT&CK | T1557 (Adversary-in-the-Middle), T1590 (Gather Victim Network Information) |
| CAPEC | CAPEC-115, CAPEC-94 |
| CVSS 4.0 | 5.9 (CVSS:4.0/AV:A/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:N/SC:N/SI:L/SA:N) |
| DREAD | 4.6 |
| Reachability | 🔴 Reachable |
| Exploit Maturity | poc |
| Detection Source | skill_scan |
Summary: The checkCorsReachable() function in offscreen.ts determines CORS acceptance purely by whether a fetch() Promise resolves rather than throws, never reading the Access-Control-Allow-Origin header value. This means a wildcard-CORS misconfigured server, or an on-path attacker's spoofed server that answers any HTTP status, will produce an identical 'pass' result to a correctly origin-scoped legitimate mediator/trust-agent endpoint.
📝 Description:
Support staff and end users viewing the wallet extension's diagnostics UI will see a green 'PASS' for 'Trust agent REST accepts this wallet's origin' even when: (a) the backend is misconfigured with wildcard CORS (a real security weakness this test is meant to catch), or (b) an on-path attacker is intercepting and answering the request. This undermines the diagnostic's stated purpose and could delay detection of a genuinely compromised or misconfigured trust-agent connection central to the wallet's DID-based trust model.
🧪 Proof of Concept:
The function's only success criterion is that the fetch Promise resolves (does not throw). It never inspects response.status or response.headers.get('Access-Control-Allow-Origin'), so it cannot distinguish 'server explicitly allowed THIS extension's origin' from 'server allows any origin' or 'server answered anything at all because it was spoofed/misconfigured'.
async function checkCorsReachable(
url: string,
fetchImpl: typeof fetch,
): Promise<{ ok: boolean; error?: unknown }> {
try {
await fetchImpl(url, { method: "GET", cache: "no-store" });
return { ok: true };
} catch (err: unknown) {
return { ok: false, error: err };
}
}
Vulnerable lines: 736, 743
🔎 Evidence: packages/extension/src/offscreen.ts:736
async function checkCorsReachable(
url: string,
fetchImpl: typeof fetch,
): Promise<{ ok: boolean; error?: unknown }> {
try {
await fetchImpl(url, { method: "GET", cache: "no-store" });
return { ok: true };
💥 Impact:
Support staff and end users viewing the wallet extension's diagnostics UI will see a green 'PASS' for 'Trust agent REST accepts this wallet's origin' even when: (a) the backend is misconfigured with wildcard CORS (a real security weakness this test is meant to catch), or (b) an on-path attacker is intercepting and answering the request. This undermines the diagnostic's stated purpose and could delay detection of a genuinely compromised or misconfigured trust-agent connection central to the wallet's DID-based trust model.
Confidentiality: Low - a wildcard-CORS-misconfigured real backend (masked by this flaw) could allow arbitrary web origins to read wallet-related API responses. · Integrity: Low - user/support trust decisions about the legitimacy of the mediator connection may be based on a false positive. · Availability: None
🧭 Reachability:
- Network exposure: public
- Auth barrier: none
- Attack path: EP-001/EP-002 (runDiagnostics(vtaDid), no auth) → diagnoseMediator() → checkCorsReachable(url, fetchImpl) at offscreen.ts:736-743 → fetchImpl GET to attacker-influenceable/on-path-interceptable host → result rendered as 'pass' in DiagnosticsReport UI
⚖️ Triage Factors:
| Factor | Value |
|---|---|
| Fixable | ✅ Yes |
| Exploitability | medium |
| Business impact | medium |
| Public exploit | |
| Environment | unknown |
Attack scenario: An on-path attacker (rogue AP/DNS spoof) or a misconfigured wildcard-CORS backend causes the diagnostics self-test to falsely report 'PASS' for CORS origin acceptance, because the code only checks whether fetch resolved, not the actual CORS header content.
🔧 Remediation:
⚠️ AI-generated fix. Review, test in staging, and validate against your architecture before applying.
The fix reads the actual Access-Control-Allow-Origin header and compares it against the expected extension origin, distinguishing origin-specific acceptance from a permissive wildcard or an unrelated resolved response. This restores the diagnostic's actual security value: proving the backend explicitly trusts this extension's origin rather than merely 'answered something.'
Vulnerable code:
async function checkCorsReachable(
url: string,
fetchImpl: typeof fetch,
): Promise<{ ok: boolean; error?: unknown }> {
try {
await fetchImpl(url, { method: "GET", cache: "no-store" });
return { ok: true };
} catch (err: unknown) {
return { ok: false, error: err };
}
}
Secure code:
async function checkCorsReachable(
url: string,
fetchImpl: typeof fetch,
expectedOrigin: string,
): Promise<{ ok: boolean; originVerified: boolean; error?: unknown }> {
try {
const res = await fetchImpl(url, { method: "GET", cache: "no-store" });
const acao = res.headers.get("Access-Control-Allow-Origin");
const originVerified = acao === expectedOrigin || acao === "*" ? acao !== "*" : false;
// Treat wildcard as NOT origin-specific verification; log separately for visibility.
return { ok: true, originVerified };
} catch (err: unknown) {
return { ok: false, originVerified: false, error: err };
}
}
Additional recommendations:
- Add a deliberate control probe using a mismatched/bogus Origin to differentiate wildcard-accept-all endpoints from origin-specific ones.
- Pin/verify TLS certificate identity for known mediator/trust-agent hosts to reduce on-path spoofing risk.
- Surface origin-verification result (specific vs wildcard vs unverifiable) distinctly in the diagnostics UI rather than a single binary pass/fail.
Also flagged at this location (same code, other weakness framings): Diagnostic Probe Has No Response-Authenticity Check, Enabling On-Path Spoofing of Trust-Agent PASS Result
🔍 Validation Log
- Verdict:
⚠️ Must-Review-By-Human- Confidence: 90%
- AI Validation Evidence: EVIDENCE FOUND: The reported code in packages/extension/src/offscreen.ts, checkCorsReachable(), does
await fetchImpl(url, { method: "GET", cache: "no-store" }); return { ok: true };inside a try/catch with no inspection of response.status or Access-Control-Allow-Origin headers. This matches the finding's description exactly. EVIDENCE NOT FOUND: offscreen.ts is not included in source_files, so I cannot see the full surrounding context (e.g. diagnoseMediator, runDiagnostics, or the design-ration- Validation Effort: This finding was validated up to a point, but the available evidence was insufficient for a conclusive automated verdict. A human (developer / security team) must manually review the code and decide. Not dismissed — treat as an open item pending human review.
🔵 Loss of Diagnostic Telemetry (HTTP Status) Weakens Incident Forensics and Repudiation Resistance
| Field | Detail |
|---|---|
| Severity | LOW |
| Location | packages/extension/src/offscreen.ts:726 |
| Finding ID | github_pr-f13d391e3ae0 |
| CWE | CWE-778, CWE-223 |
| OWASP | A09:2021 - Security Logging and Monitoring Failures |
| MITRE ATT&CK | T1070 (Indicator Removal) |
| CAPEC | CAPEC-454 |
| CVSS 4.0 | 2.3 (CVSS:4.0/AV:L/AC:L/AT:N/PR:H/UI:N/VC:N/VI:N/VA:N/SC:N/SI:L/SA:N) |
| Reachability | 🔴 Reachable |
| Exploit Maturity | theoretical |
| Detection Source | skill_scan |
🔎 Evidence: packages/extension/src/offscreen.ts:726
): Promise<{ ok: boolean; error?: unknown }> {
try {
await fetchImpl(url, { method: "GET", cache: "no-store" });
return { ok: true };
} catch (err: unknown) {
return { ok: false, error: err };
}
🧭 Reachability:
- Network exposure: public
- Auth barrier: none
- Attack path: EP-001/EP-002 → checkCorsReachable() (status discarded at offscreen.ts:741, previously captured at removed line
return { ok: true, status: res.status }) → diagnoseMediator/runDiagnostics render status-agnostic 'pass' text
⚖️ Triage Factors:
| Factor | Value |
|---|---|
| Fixable | ✅ Yes |
| Exploitability | low |
| Business impact | low |
| Public exploit | None known |
| Environment | unknown |
Attack scenario: This is primarily a security-monitoring/forensics degradation: during a concurrent incident (e.g., active MITM causing an anomalous 500/200/403 status instead of the expected 405), the removed status field means engineers cannot distinguish expected-benign non-2xx responses from anomalous ones via the diagnostics UI alone.
🔍 Validation Log
- Verdict:
⚠️ Must-Review-By-Human- Confidence: 90%
- AI Validation Evidence: EVIDENCE FOUND: The evidence snippet shows checkCorsReachable's return type is
Promise<{ ok: boolean; error?: unknown }>with nostatusfield, consistent with the claim that a previously-captured HTTP status was removed from the return value and from rendered diagnostic strings. EVIDENCE NOT FOUND: The actual prior version of the code (to diff against) and the diagnoseMediator/runDiagnostics consumer code that renders the 'detail' strings are not present in source_files, so I cannot directly- Validation Effort: This finding was validated up to a point, but the available evidence was insufficient for a conclusive automated verdict. A human (developer / security team) must manually review the code and decide. Not dismissed — treat as an open item pending human review.
Generated by Agentic Sec — AI Security Validation Agent
This report includes full scan data + AI validation evidence. Feed to engineering copilots for automated fix deployment.
Details
🛡️ Threat Model & Affect Analysis — PR #152
| Field | Value |
|---|---|
| Repository | OpenVTC/vta-browser-plugin |
| Branch | fix/self-test-drop-status-codes → main |
| Generated | 2026-09-05 |
ℹ️ This report contains theoretical threats and impact analysis for the MR.
Unlike the Security Code Review Report (which contains confirmed, materialised issues),
these are potential risks that may or may not be exploitable. Use this for defence-in-depth planning.
📋 Affect Analysis
Change Summary
Removes HTTP status codes from the return type of checkCorsReachable() and from two user-facing diagnostic 'detail' strings (in diagnoseMediator and runDiagnostics) so that a passing CORS self-test no longer displays a raw HTTP status (e.g. 'PASS ... HTTP 405') that reads as a contradiction to a human reviewer. The underlying CORS-acceptance decision logic (any resolved response = pass, thrown error = fail) is unchanged.
Diff: +22 / -9 lines
Types: diagnostics, refactor, documentation
Risk Assessment
- Overall Risk: low
- Review Priority: before_merge
- Pentest Needed: false
- Security Review Needed: true
This is a small, low-risk, diagnostics-messaging-only change confined to a single file. It does not alter the underlying CORS-acceptance security decision (any resolved fetch = pass, thrown error = fail), does not introduce new attack surface, does not touch authentication/authorization, secrets, or network request construction. The primary residual concerns are (1) a minor breaking change to an internal TypeScript return type that requires confirming no other consumers depend on the removed field, and (2) a reduction in diagnostic/forensic fidelity (status code no longer captured anywhere, not just hidden from the UI) which is a legitimate but low-severity trade-off. Standard code review (not a security pentest) is warranted primarily to confirm completeness of the refactor across the codebase, which is outside the scope of what this reduced single-file diff can verify.
Review Focus Areas:
- Confirm no other consumers in the repository (outside this single-file diff) depend on the removed
statusfield from checkCorsReachable()'s return type. - Confirm the underlying CORS accept/reject decision logic (unchanged by this PR, per prior analysis) is not itself weakened — this PR does not touch it, but reviewers should independently verify rather than relying solely on the embedded comment narrative, per prior threat-model observations about persuasive inline comments in this file.
- Confirm no automated tooling or support workflows parse the removed HTTP status out of the 'detail' strings for alerting/monitoring purposes.
⚠️ Security Implications
🔵 Loss of HTTP status code visibility in CORS self-test diagnostics
Loss of HTTP status code visibility in CORS self-test diagnostics
Action: Retain the status code in an internal debug/telemetry log (not the user-facing string) so forensic capability is preserved without reintroducing the confusing 'PASS ... HTTP 405' UX issue this PR fixes.
⚪ Improved clarity of security self-test PASS messaging
Improved clarity of security self-test PASS messaging
Action: No action required; consider applying the same clarity review to any other diagnostic messages elsewhere in the codebase that similarly juxtapose a PASS/FAIL label with a raw status code.
🔵 Breaking change to internal function contract (checkCorsReachable return type)
Breaking change to internal function contract (checkCorsReachable return type)
Action: Search the full repository for other usages of checkCorsReachable() and any serialization/consumption of the DiagnosticsReport status field to confirm completeness of this refactor.
🧩 Affected Components
| Component | Impact | Change | What Changed |
|---|---|---|---|
| Offscreen Diagnostics Module (CORS Self-Test) | low | modified | The internal CORS-reachability probe function no longer returns or exposes an HTTP status code, and the two consumer functions that build us |
📁 File Classifications
packages/extension/src/offscreen.ts
- Type: security
💡 Recommendations
- SHOULD — Preserve the HTTP status code in an internal debug/telemetry log, separate from the user-facing PASS detail string. (effort: small)
- Avoids losing forensic/audit capability for anomalous-but-non-throwing responses while still fixing the confusing PASS+status-code UX issue.
- SHOULD — Search the full repository for any other consumers of checkCorsReachable()'s return value or of the DiagnosticsReport 'status' field to confirm this refactor is complete. (effort: small)
- Only a reduced single-file diff was available for analysis; the type change is breaking and other consumers cannot be ruled out from the provided evidence.
- CONSIDER — Add/update unit tests asserting the new return shape of checkCorsReachable() and the new wording of the PASS detail strings. (effort: small)
- No test changes accompany this diff; a signature change and two string changes are otherwise unverified by automated coverage.
- CONSIDER — Trim the extended first-person rationale comments to concise technical notes, moving detailed justification to the PR description. (effort: small)
- Long persuasive narrative comments in security-relevant code are an atypical pattern that can reduce reviewer scrutiny of future changes if the style is repeated at scale, per prior threat-model observation (informational-level, precedent-based concern).
✅ Positive Observations
- Fixes a genuine UX/trust problem where a passing security self-test displayed a raw 4xx status code next to a green PASS label, which read as a contradiction to reviewers.
- The underlying security-relevant accept/reject logic in checkCorsReachable() is left completely untouched by this change — no weakening of the actual CORS-acceptance determination occurs.
- The runDiagnostics() REST-check detail string fix also restores a previously-dropped clause ('with this extension's origin'), improving message completeness beyond just removing the status code.
- Both known call sites of the modified function were updated consistently within the same diff, avoiding an obviously broken half-migration.
- The change is small, isolated, and easy to review in full (single file, three hunks).
🛡️ STRIDE Threat Model
Identified Threats (5)
🔵 STRIDE-1: Diagnostic Status Suppression in checkCorsReachable
| Field | Detail |
|---|---|
| Category | Repudiation, Information Disclosure |
| Severity | Low |
| Likelihood | Possible |
| CVSS | 2.3 CVSS:4.0/AV:L/AC:L/AT:N/PR:H/UI:N/VC:N/VI:N/VA:N/SC:N/SI:L/SA:N |
| Residual Severity | Low |
| CWE | CWE-778,CWE-223 |
| CAPEC | CAPEC-454 |
| OWASP | A09:2021 - Security Logging and Monitoring Failures |
Description: checkCorsReachable in offscreen.ts allows loss of diagnostic fidelity due to removal of the HTTP status code from the returned result, resulting in reduced troubleshooting/repudiation capability for security self-test failures that manifest as ambiguous non-error responses
Evidence: packages/extension/src/offscreen.ts:~731-738
async function checkCorsReachable(
url: string,
fetchImpl: typeof fetch,
): Promise<{ ok: boolean; error?: unknown }> {
try {
await fetchImpl(url, { method: "GET", cache: "no-store" });
return { ok: true };
} catch (err: unknown) {
return { ok: false, error: err };
}
Attack Scenario:
- A misconfigured or maliciously altered mediator/trust-agent server begins returning an unexpected status (e.g., 500, 403 due to WAF, or a captive-portal 200) instead of the expected CORS-allow response.
- checkCorsReachable() in packages/extension/src/offscreen.ts only returns
{ ok: true }or{ ok: false, error }, discardingres.statusthat was previously captured viaconst res = await fetchImpl(url, ...). - diagnoseMediator() and runDiagnostics() consume this result and render a 'pass' status with detail text
"${host} answered a request carrying this extension's origin."with no numeric status. - A support engineer or the end user reviewing the self-test UI cannot distinguish a legitimate 405 (expected) from an anomalous 200/500 response that might indicate a proxy/MITM intercepting and answering on behalf of the real endpoint.
- Because the detail string is now status-agnostic, an attacker who stands up a rogue endpoint answering any HTTP status with permissive CORS headers produces an identical-looking 'pass' diagnostic, making anomaly detection by a human operator harder and reducing the evidentiary trail for later incident analysis.
🔎 Threat Clue: Derived from COMP-001 via EP-001, EP-002
- Data Flows: fetchImpl(url) -> checkCorsReachable -> diagnoseMediator/runDiagnostics -> DiagnosticsReport UI
Preconditions: Attacker or network condition must be able to influence the response received by fetchImpl for the mediator/trust-agent URL (e.g., on-path attacker, DNS/host misconfiguration, malicious relay)., Diagnostics UI must be relied upon by a human as the sole source of truth for CORS-acceptance troubleshooting.
Existing Controls: Failures still carry detail and a code field per the surrounding code comments, preserving diagnosability for the failure path. • The underlying accept/reject determination logic (thrown fetch error = fail, any resolved response = pass) is unchanged, so the security decision itself is not weakened.
Recommended Mitigations: Retain the numeric status internally for logging/telemetry purposes even if not shown in the user-facing pass message. • Add a verbose/debug diagnostics mode that surfaces the raw status code and headers to engineers without cluttering the standard user-facing pass string. • Emit a structured audit log entry (status, host, timestamp) server-side or in extension storage for every diagnostic run to support later repudiation-resistant review.
🟡 STRIDE-2: CORS-Reachability-As-Security-Test Logic Flaw in checkCorsReachable
| Field | Detail |
|---|---|
| Category | Spoofing, Information Disclosure |
| Severity | Medium |
| Likelihood | Possible |
| CVSS | 5.3 CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:N/SC:N/SI:L/SA:N |
| Residual Severity | Medium |
| CWE | CWE-346,CWE-942 |
| CAPEC | CAPEC-115 |
| OWASP | A05:2021 - Security Misconfiguration |
Description: checkCorsReachable in the offscreen document allows a false sense of assurance due to conflating 'received any HTTP response' with 'origin was accepted by CORS policy', resulting in misleading 'pass' diagnostics when the browser's CORS enforcement is bypassed or absent (e.g. this code running with elevated/extension privileges rather than page-level fetch)
Evidence: packages/extension/src/offscreen.ts:~724-742
async function checkCorsReachable(
url: string,
fetchImpl: typeof fetch,
): Promise<{ ok: boolean; error?: unknown }> {
try {
await fetchImpl(url, { method: "GET", cache: "no-store" });
return { ok: true };
}
Attack Scenario:
- The design rationale documented in the comments assumes 'reading any status at all proves the origin was allowed' because a normal webpage fetch under standard CORS enforcement would throw/opaque-fail on disallowed origins.
- However, fetch calls originating from a privileged extension context (background/offscreen document) can behave differently than page-context fetch with respect to CORS depending on host permissions declared in the extension manifest, and cross-extension/cross-origin requests may bypass the assumption entirely if the extension holds broad host permissions.
- If the extension's manifest.json grants
<all_urls>or wildcard host permissions,fetchImpl(built-in fetch or a wrapped variant) run inside the offscreen document may succeed in ways that do not reflect what a normal web page would experience, meaningcheckCorsReachablereturningok: truedoes not reliably prove the target server'sAccess-Control-Allow-Originaccepted the extension's origin specifically. - A rogue or compromised mediator/trust-agent endpoint that simply accepts all origins (misconfigured wildcard CORS) will report as 'pass' identically to a properly-scoped endpoint, since the probe records no header/status detail to distinguish a wildcard-accept from an origin-specific accept.
- This can produce a false-positive security posture in the diagnostics UI (e.g. 'Trust agent REST accepts this wallet's origin: PASS') even though the endpoint may actually be permissively misconfigured to accept any origin, which is itself the security weakness the diagnostic was meant to catch.
🔎 Threat Clue: Derived from COMP-001 via EP-001, EP-002
- Data Flows: fetchImpl(url) -> checkCorsReachable -> diagnoseMediator -> DiagnosticsReport
Preconditions: Target mediator/trust-agent endpoint is misconfigured with a permissive/wildcard CORS policy., Diagnostics output is trusted by developers/support as proof of correctly scoped CORS rather than merely reachability.
Existing Controls: The comment block explicitly documents the intended semantics and threat model of the probe, aiding future maintainers. • Failure path (network/CORS rejection) still throws and is captured distinctly as ok: false.
Recommended Mitigations: Inspect and record the actual Access-Control-Allow-Origin response header value (where readable) rather than inferring acceptance solely from absence of a thrown error. • Explicitly test with a second, deliberately-mismatched origin/control probe to differentiate 'accepts this origin' from 'accepts any origin'. • Document in the diagnostics report whether the observed acceptance is origin-specific or wildcard-based.
🔵 STRIDE-3: Unauthenticated GET Probe Enables Third-Party Endpoint Fingerprinting via diagnoseMediator
| Field | Detail |
|---|---|
| Category | Denial of Service, Information Disclosure |
| Severity | Low |
| Likelihood | Unlikely |
| CVSS | 3.1 CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:N/SC:N/SI:N/SA:L |
| Residual Severity | Low |
| CWE | CWE-918,CWE-799 |
| CAPEC | CAPEC-664 |
| OWASP | A10:2021 - Server-Side Request Forgery (SSRF) |
Description: GET request in checkCorsReachable against attacker-influenceable mediator/trust-agent URLs allows unauthenticated reachability/response-timing probing due to absence of rate limiting or destination allow-listing, resulting in the extension being usable as a blind SSRF-style probe against arbitrary reachable hosts if vtaDid or restUrl derivation is attacker-influenced
Evidence: packages/extension/src/offscreen.ts:~910-930
async function runDiagnostics(vtaDid: string): Promise<DiagnosticsReport> {
...
detail: `${originOf(restUrl) ?? restUrl} answered a request carrying this extension's origin.`,
...
} else {
const reachable = await probeReachable(restUrl, fetchImpl);
Attack Scenario:
- runDiagnostics(vtaDid) in offscreen.ts derives a restUrl and/or mediator URL, ultimately passed into checkCorsReachable(url, fetchImpl) as the
urlparameter (see EP-001/EP-002). - If
vtaDidor a derived URL value is influenced by attacker-controlled data (e.g., a malicious DID document, a crafted wallet configuration, or a compromised mediator registry entry) that this diff's visible code does not fully validate, the extension could be induced to issue outbound GET requests to arbitrary attacker-chosen hosts. - Each diagnostics run issues one or more
fetchImpl(url, { method: 'GET', cache: 'no-store' })calls with no visible rate limiting, timeout ceiling, or destination allow-list enforcement in the reduced source shown. - An attacker who can trigger repeated diagnostics runs (e.g., via a script embedded in a connecting dApp/page that repeatedly asks the wallet to re-run health checks) could use the extension as a low-volume network reachability oracle or amplification helper against third-party hosts, or induce excessive outbound traffic from the user's browser.
- Because the probe intentionally discards status codes and does not log destination or timing centrally, abuse of this mechanism as a scanning primitive would leave minimal forensic trail (compounds with STRIDE-1).
🔎 Threat Clue: Derived from COMP-001 via EP-001, EP-002
- Data Flows: vtaDid -> runDiagnostics -> restUrl/mediator URL -> checkCorsReachable -> fetchImpl -> external host
Preconditions: vtaDid or URL construction logic (not visible in this reduced diff) must allow attacker-influenced input to reach the url argument of checkCorsReachable/fetchImpl., Diagnostics function must be triggerable by a remote/web page context with enough frequency to be useful for probing or DoS.
Existing Controls: cache: "no-store" prevents caching-based amplification but does not limit request rate. • This is a diagnostics/self-test path, likely only user-triggered rather than automatically exposed to arbitrary web pages, reducing realistic exploitability.
Recommended Mitigations: Validate and allow-list the set of permissible mediator/trust-agent hosts before invoking checkCorsReachable. • Apply per-origin/per-session rate limiting to diagnostics invocation. • Restrict which extension contexts/messages are permitted to trigger runDiagnostics().
⚪ STRIDE-4: Embedded Narrative Comments as Prompt-Injection-Style Content in Source Diagnostics Module
| Field | Detail |
|---|---|
| Category | Repudiation, Tampering |
| Severity | Informational |
| Likelihood | Very Unlikely |
| CVSS | 1.0 CVSS:4.0/AV:L/AC:L/AT:N/PR:H/UI:N/VC:N/VI:N/VA:N/SC:N/SI:N/SA:N |
| Residual Severity | None |
| CWE | CWE-1059 |
| CAPEC | CAPEC-437 |
| OWASP | A08:2021 - Software and Data Integrity Failures |
Description: Large first-person justificatory comment blocks in offscreen.ts allow social-engineering-style misdirection of future human reviewers or AI-assisted code-review tooling due to unusually persuasive/conversational comment style embedded directly in security-relevant diagnostic code, resulting in a risk that reviewers accept security-relevant behavioral changes (status code removal) without independent verification
Evidence: packages/extension/src/offscreen.ts:~724-742, ~780-796
// No status code on a pass. It used to name one, reasoning that seeing
// the number would make it obvious a 4xx is expected and not the thing
// being tested. It did the opposite...
Attack Scenario:
- The diff replaces terse rationale comments with long, first-person, argumentative prose (e.g., 'It is therefore not returned: nothing can do anything useful with it...') directly inside a security self-test module in packages/extension/src/offscreen.ts.
- This style is unusual for the surrounding codebase and reads as though it is trying to preemptively argue a human or automated reviewer out of flagging the change, rather than concisely documenting behavior.
- A future contributor (human or an LLM-assisted review/agent) skimming the diff may be persuaded by the embedded prose to accept the removal of
statusfrom the returned diagnostic object without independently re-deriving whether status-code visibility has any legitimate security/debugging value. - If this pattern is repeated at scale across a codebase (comments engineered to pre-empt scrutiny), it could be used by a malicious insider or supply-chain-compromised contributor to slip in substantive security-relevant behavior changes disguised as 'just a UX wording fix', because reviewers are anchored by the narrative rather than the diff mechanics.
- In this specific instance the change itself appears benign (UI wording plus dropping an unused field), but the pattern and precedent are the risk being flagged, not this individual commit.
🔎 Threat Clue: Derived from COMP-001 via EP-001, EP-002
- Data Flows: Source code review pipeline -> merged commit -> shipped extension
Preconditions: Code review process relies on comment narrative rather than independent verification of behavioral diff., Repeated or scaled use of this commenting style across a codebase by the same or coordinated contributors.
Existing Controls: The change is small, isolated to one file, and the actual security-decision logic (accept/reject) is unchanged as confirmed by prior analysis. • Version control retains full diff history so the change is auditable regardless of comment framing.
Recommended Mitigations: Enforce code review standards that require justification for security-relevant field removals to be validated against tests/telemetry rather than accepted from comment prose alone. • Treat unusually long persuasive comments in security-sensitive files as a review flag warranting extra scrutiny. • Require a linked issue/ADR for changes to diagnostic/security self-test semantics rather than inline narrative-only justification.
🟡 STRIDE-5: Missing Origin-Specificity Verification Enables On-Path Response Spoofing in Diagnostic Probe
| Field | Detail |
|---|---|
| Category | Spoofing, Tampering |
| Severity | Medium |
| Likelihood | Possible |
| CVSS | 5.9 CVSS:4.0/AV:A/AC:L/AT:N/PR:N/UI:N/VC:N/VI:L/VA:N/SC:N/SI:L/SA:N |
| Residual Severity | Low |
| CWE | CWE-290,CWE-300 |
| CAPEC | CAPEC-94 |
| OWASP | A07:2021 - Identification and Authentication Failures |
Description: checkCorsReachable's GET probe over plain reachability allows response spoofing/impersonation by an on-path or malicious DNS/network actor due to no TLS pinning, no response-integrity check, and no distinction between a real endpoint and a spoofed one that merely answers with any status, resulting in the diagnostics UI reporting a false 'pass' for CORS acceptance against a spoofed trust-agent/mediator endpoint
Evidence: packages/extension/src/offscreen.ts:~731-738
try {
await fetchImpl(url, { method: "GET", cache: "no-store" });
return { ok: true };
} catch (err: unknown) {
return { ok: false, error: err };
}
Attack Scenario:
- An attacker positioned on the network path (malicious Wi-Fi AP, compromised router, or DNS hijack) intercepts the GET request issued by
fetchImpl(url, { method: 'GET', cache: 'no-store' })toward the mediator or trust-agent REST URL. - The attacker's spoofed server answers with any HTTP response (even a 4xx/5xx) without needing to know any secret, satisfying the only success criterion in checkCorsReachable: that the fetch promise resolves rather than throws.
- checkCorsReachable() returns
{ ok: true }; diagnoseMediator()/runDiagnostics() render a 'pass' status stating the host 'answered a request carrying this extension's origin', which a user or support engineer interprets as confirmation of a legitimate, correctly-configured trust agent. - Because the response body/headers proving genuine CORS/Access-Control-Allow-Origin enforcement from the real backend are never inspected (only 'did it resolve'), the spoofed endpoint passes the self-test identically to the real one.
- The user, believing the diagnostics 'PASS', continues trusting the wallet/mediator connection, potentially proceeding with credential exchange or DID-based operations against what could actually be a spoofed intermediary if other parts of the connection flow (not shown in this reduced diff) also lack transport-integrity verification.
🔎 Threat Clue: Derived from COMP-001 via EP-001, EP-002
- Data Flows: Network path (attacker-controllable) -> fetchImpl -> checkCorsReachable -> diagnoseMediator/runDiagnostics -> DiagnosticsReport UI -> User trust decision
Preconditions: Attacker must have on-path network position (rogue AP, ARP/DNS spoof, malicious proxy) or control of DNS resolution for the mediator/trust-agent hostname., Downstream wallet/DID flows must place meaningful trust weight on the diagnostics 'pass' result.
Existing Controls: Standard TLS/HTTPS transport (assumed, not shown) would prevent trivial on-path tampering unless the attacker also controls a trusted certificate or the user accepts a MITM certificate warning. • This is a self-diagnostic UX signal, not itself an authentication or authorization gate for the underlying wallet protocol, limiting direct exploitability.
Recommended Mitigations: Verify TLS certificate/hostname pinning for known mediator/trust-agent endpoints before treating a diagnostic probe as a pass. • Cross-check the Access-Control-Allow-Origin response header value against the expected extension origin rather than treating any resolved response as sufficient. • Clearly scope the diagnostics feature in documentation/UI as a reachability check, not a full authenticity assurance, to prevent over-trust by users.
🍝 PASTA Threat Model
Application Purpose
A browser extension (VTA wallet plugin) that mediates trust-agent/DID (decentralized identity) wallet interactions and provides an in-extension diagnostics self-test to verify that mediator and trust-agent REST endpoints correctly accept the extension's origin via CORS, giving users/support confidence the wallet's connectivity is correctly configured.
Inherent Risks
- Diagnostics logic treats mere response resolution as proof of correct origin-scoped CORS acceptance rather than verifying actual header values.
- Extension runs in a privileged browser context where fetch-based CORS assumptions valid for normal web pages may not hold identically.
- No visible allow-list or validation on destination URLs passed into the diagnostic fetch probe within the reduced source shown.
Objectives
Risk: Avoid false-positive security assurances that could lead users to trust a spoofed or misconfigured mediator/trust-agent endpoint.
Business: Provide users and support staff with a reliable self-service diagnostic to confirm wallet-to-mediator/trust-agent connectivity.
Security: Ensure diagnostics cannot be leveraged as a probing primitive against arbitrary third-party hosts.; Ensure diagnostic PASS results reflect genuine, origin-specific CORS acceptance and not merely 'the network resolved a response'.
Financial: Minimize support costs associated with misdiagnosed connectivity issues.
Compliance: Maintain auditability of diagnostic decisions to support incident investigation for decentralized-identity trust operations.
Functional: Accurately determine whether a mediator/trust-agent endpoint accepts the extension's origin under CORS.
Operational: Keep diagnostic UI messaging unambiguous so PASS/FAIL results are trusted and actionable by non-expert users.
Business Impact Analysis (1)
BIA-1: Wallet Connectivity Self-Diagnostics (Medium)
End-to-end process where a user or support engineer runs in-extension diagnostics to confirm the wallet's mediator and trust-agent REST endpoints correctly accept the extension's origin before relying on DID-based wallet operations.
MTD: 01 days 00:00 hours | RTO: 00 days 04:00 hours | RPO: 00 days 00:00 hours
- Stakeholders: End Users / Extension Maintainers / Support Engineers / Trust Agent Operators
- Dependencies: Browser Fetch API / Mediator Service / Offscreen Document Runtime / Trust Agent REST Service
- Disruptions: Diagnostics reporting false PASS against a spoofed or misconfigured endpoint / Diagnostics reporting false FAIL due to transient network issues causing unnecessary support escalations / Excessive or abusive triggering of diagnostics causing outbound request flooding
- Impacts: Users proceed to use a misconfigured or spoofed wallet backend believing it is verified, risking DID/credential exposure / Increased support burden from ambiguous or misleading diagnostic messaging / Reduced ability to forensically reconstruct what a diagnostics run actually observed, due to discarded status codes
Technical Scope
Roles (3): RO-1 Extension End User · RO-2 Support Engineer · RO-3 Extension Maintainer
Actors (3): AC-1 End User Browser Session · AC-2 Offscreen Document Runtime · AC-3 fetchImpl Network Client
Use Cases (1): Wallet Connectivity Self-Diagnostics
Attack Trees (1): SC-1: Offscreen Diagnostics Module
Entry Points (2): EP-1 CORS Reachability Probe · EP-2 Run Diagnostics Invocation
Risk Registry (4): RISK-001 · RISK-002 · RISK-003 · RISK-004
Threat Actors (3): TA-1 On-Path Network Attacker · TA-2 Malicious Web Page / dApp · TA-3 Malicious or Compromised Contributor
Infrastructure (2): IF-1 User's Browser Extension Runtime · IF-2 Mediator/Trust Agent Hosting Infrastructure
Trust Boundaries (2): TB-1 Extension-to-External-Network Boundary · TB-2 Extension Internal Runtime Boundary
External Entities (2): EE-1 Mediator Server · EE-2 Trust Agent REST Server
System Components (4): SC-1 Offscreen Diagnostics Module · SC-2 Mediator Service · SC-3 Trust Agent REST Service · SC-4 Diagnostics Report UI
Resources And Assets (2): RA-1 Diagnostics Report · RA-2 Mediator/Trust Agent URL Configuration
Technologies And Dependencies (3): TD-1 WebExtension Offscreen Document API · TD-2 Fetch API · TD-3 TypeScript
⚔️ Attack Scenarios (1)
Exploit identified weaknesses
flowchart LR
S0["Diagnostic Status Suppression in checkCorsReachable"]
S1["CORS-Reachability-As-Security-Test Logic Flaw in checkCorsRe"]
S2["Unauthenticated GET Probe Enables Third-Party Endpoint Finge"]
S0 --> S1
S1 --> S2
📊 Risk Summary
Total Threats: 5
By Severity: Low: 2 · Medium: 2 · Informational: 1
By Category: Repudiation: 2 · Information Disclosure: 3 · Spoofing: 2 · Denial of Service: 1 · Tampering: 2
Generated by Agentic Sec — Threat Model & Affect Analysis Agent
📊 Summary & findings
| ✅ Confirmed | |
|---|---|
| 0 | 3 |
Must-Review-By-Human (3)
- 🟡 CORS Reachability Probe Conflates 'Response Received' with 'Origin Validated' (Missing Header Verification)
- 🟡 Diagnostic Probe Has No Response-Authenticity Check, Enabling On-Path Spoofing of Trust-Agent PASS Result
- 🔵 Loss of Diagnostic Telemetry (HTTP Status) Weakens Incident Forensics and Repudiation Resistance
2 participants
Before / after
Why
Reported from the panel: "why is this showing 4xx errors?" Nothing was wrong.
The status was named on purpose — the comment said so: naming it would "make it obvious to a reader that a 4xx is expected and is not the thing being tested". It did the opposite. A green PASS beside a 405 reads as a contradiction, and resolving it needs the reader to already understand CORS probing, which is exactly the knowledge this panel exists to not require. A comment predicting the confusion is not the same as preventing it.
checkCorsReachablesends a plainGETagainst the same endpoint that would fail — no custom headers, so no preflight, so the browser checksAccess-Control-Allow-Originon the real response. A POST-only auth route answers 405; a REST base with no handler at its bare path answers 404. Both are complete passes: any status proves the origin was allowed, because a CORS refusal has no status to read at all.So the number answers a question nobody asked, in a place where an unexplained number reads as a fault.
Scope
checkCorsReachableno longer returnsstatus. Its only consumer was that misleading line, and a field kept for a future caller who would be wrong to use it is not worth keeping.code(R3.7) — that is where the status-shaped information is actually diagnostic.Cosmetic; no behaviour change. Lint, build and 682 tests pass.