Skip to content

fix: bypass default-browser PATH hijack on Windows for --private @W-23807283@ - #1748

Open
WillieRuemmele wants to merge 8 commits into
mainfrom
wr/fixPrivateBrowserPathHijack
Open

fix: bypass default-browser PATH hijack on Windows for --private @W-23807283@#1748
WillieRuemmele wants to merge 8 commits into
mainfrom
wr/fixPrivateBrowserPathHijack

Conversation

@WillieRuemmele

@WillieRuemmele WillieRuemmele commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • On Windows, sf org open --private calls the default-browser npm package which invokes bare reg (no fully-qualified path), allowing a malicious repo-local reg.exe to execute via PATH hijack
  • This PR bypasses default-browser entirely on Windows by detecting the default browser ourselves using a fully-qualified %SYSTEMROOT%\System32\reg.exe path, then passing an explicit app name + incognito flag to open() (which skips default-browser for any concrete app name)
  • Mirrors the safe pattern from wsl-utils's powerShellPath() which already qualifies system binary paths
  • Upstream fix contributed and merged: Use fully-qualified path for reg.exe on Windows sindresorhus/default-browser#23 (released in default-browser@5.5.1)
  • Bumped open to ^11.0.1 to pull in the upstream fix via lockfile; workaround kept as defense-in-depth

Work Item

@W-23807283@: SF CLI - Project-local reg.exe leads to RCE during private org opening

Test plan

  • Unit tests for getWindowsPrivateBrowserApp() covering Chrome, Firefox, Edge, Brave, hyphen-suffixed ProgIds, unsupported browsers, and unparseable output
  • Unit test verifying the fully-qualified path uses SYSTEMROOT env var
  • Windows-only NUT that verifies a poisoned PATH does not result in execution of a local reg.exe
  • Manual verification on Windows with a repo containing node_modules/.bin/reg.cmd

@k80bowman k80bowman left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this, though I do have a comment on one of the tests.

Comment thread test/nut/openPrivatePathHijack.nut.ts Outdated
fs.mkdirSync(poisonBin, { recursive: true });

fs.writeFileSync(
path.join(poisonBin, 'reg.cmd'),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a reg.exe file, right? The bug will not trigger on a reg.cmd file.

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