[GHSA-53mr-w429-c597 follow-up] - filesystem: close the parent-directory swap on Windows

Aperta
#5,260 3 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
25/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
github-actions, node.js, typescript
Ambito
backend, security

Direzione di ricerca

Start with src/filesystem/pinned-directory.ts and the Windows filesystem behavior traced in the issue. Review the listed design options and acceptance criteria; implementation requires choosing a native interface and may involve release pipeline work in AGENTS.md and RELEASING.md. Done means the Windows junction swap regression cases pass on the windows-latest leg of typescript.yml, the other listed filesystem behaviors remain covered, and the advisory is updated.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug server-filesystem v2

Follow-up to GHSA-53mr-w429-c597 (#5257), whose fix (#5237, @modelcontextprotocol/server-filesystem 2.0.1) closes the parent-directory swap on Linux and macOS only.

Problem

GHSA-53mr-w429-c597
is fixed on Linux and macOS but not on Windows. On Windows, a directory on
the path of a write_file, edit_file, create_directory or move_file
(destination) call can still be swapped for a junction between validation and
use. The write then lands outside the allowed directories, as it did before the
fix. Creating a junction needs no special privilege.

The fix anchors each write to its directory. It chdirs into the validated
directory, checks process.cwd(), then opens by bare name (src/filesystem/pinned-directory.ts).
That works where getcwd reports the directory the process actually entered.
On Windows it does not, and Node gives this design no handle-relative behaviour
at all.

Why the chdir anchor does nothing on Windows (traced in Node v24.20.0's source)
  1. fs.realpathSync.native and fs.openSync pass the path straight to the C++
    binding (printed from the Node 24 runtime).
  2. src/node_file.cc: RealPath (line 2094) and Open (line 2313) call
    ToNamespacedPath(env, &path) before doing anything else, and the other fs
    bindings do the same.
  3. src/path.cc:282: on Windows, ToNamespacedPath calls PathResolve, which
    prepends env->GetCwd() to a relative path and returns an absolute
    \\?\C:\… string.
  4. src/env.cc:709 → libuv uv_cwd → GetCurrentDirectoryW returns the cwd
    as a string: whatever uv_chdir passed to SetCurrentDirectoryW.
  5. libuv fs__realpath (deps/uv/src/win/fs.c) calls CreateFileW on that
    absolute string, then GetFinalPathNameByHandleW.

So every "relative" open, mkdir, rename and realpath that Node makes on
Windows is re-resolved from the cwd path string. None of them goes through the
directory handle Windows holds for the cwd. A check such as
fs.realpathSync.native('.') after chdir reports where the string points
now, so an attacker who swaps the junction back passes it.

Options

A. Check the cwd with realpathSync.native('.'): ruled out

This fails for the reason above.

B. Lock every directory on the path

Walk from the allowed root to the target. Hold each directory open so it cannot
be renamed or replaced. Check the chain only once it is locked, then operate
and release.

  • Sound under documented Windows semantics.
  • libuv only offers two share modes. It opens with all sharing modes
    (FILE_SHARE_READ | WRITE | DELETE) unless UV_FS_O_EXLOCK is passed, in
    which case it shares nothing (deps/uv/src/win/fs.c:501-509). There is no
    "deny delete only" mode.
  • Writes would be refused at random. The locks must be fully exclusive, so
    a write fails whenever Explorer, an editor, an indexer or antivirus has any
    directory on the path open.
  • Unverified: whether Node passes UV_FS_O_EXLOCK through.
    fs.constants does not expose it.
C. Open relative to a directory handle (the real fix)

Windows' equivalent of openat is NtCreateFile/NtOpenFile with
OBJECT_ATTRIBUTES.RootDirectory set to a directory handle. Win32 does not
expose it. Rust's standard library used this to fix the same class of bug,
CVE-2022-21658 in remove_dir_all: see open_link_no_reparse in
library/std/src/sys/fs/windows/remove_dir_all.rs. That function sets
RootDirectory to the parent handle and adds FILE_OPEN_REPARSE_POINT. It
also sets OBJ_DONT_REPARSE, retrying without it on INVALID_PARAMETER for
older Windows.

Design for this server:

  1. Open the allowed root as a directory handle.
  2. Walk down one component at a time. Open each component relative to the
    previous handle with FILE_OPEN_REPARSE_POINT | FILE_DIRECTORY_FILE, and
    refuse it if it carries FILE_ATTRIBUTE_REPARSE_POINT (a junction or
    symlink). Swapping a component cannot redirect the walk, because it follows
    handles, not strings.
  3. write_file (create): NtCreateFile(FILE_CREATE) relative to the
    parent handle.
  4. Overwrite and edit_file: FILE_OPEN with FILE_OPEN_REPARSE_POINT,
    then check that the opened file is not a reparse point.
  5. create_directory: NtCreateFile(FILE_DIRECTORY_FILE | FILE_CREATE) at
    each missing level, relative to the previous handle.
  6. move_file (destination): NtSetInformationFile with
    FileRenameInformation, whose RootDirectory is the destination
    directory's handle.
  7. Writing content: either through the native handle, or by turning it
    into a Node fd with ucrt's _open_osfhandle. That route is plausible but
    unverified: Node's fs on Windows maps fds through the CRT.

Known catches:

  • FAT and exFAT reject FILE_OPEN_REPARSE_POINT with INVALID_PARAMETER, so
    retry without it (see ReactOS).
  • OBJ_DONT_REPARSE exists only on newer Windows.
  • Wine historically rejected a non-null RootDirectory.

Three ways to ship option C:

How Works on Node 24 (the engines floor)? Cost
C1. Built-in node:ffi Call ntdll.dll from JS No. Added in v26.1.0 and experimental. Checked on macOS: Node 24.20 has no node:ffi; Node 26.7.0 loads it only with --experimental-ffi (newer 26.x docs say it can be disabled with --no-experimental-ffi). No struct type, so OBJECT_ATTRIBUTES, UNICODE_STRING and IO_STATUS_BLOCK are hand-built in Buffers. A wrong signature crashes the process. Passing a flag to an npx bin on Windows is fragile. Restricted under the permission model unless --allow-ffi is set.
C2. Third-party FFI (e.g. koffi) Prebuilt FFI with struct support, as a win32-only optional dependency Yes Quickest to build, but puts a third-party native binary inside an official reference package. A supply-chain decision.
C3. Own N-API addon Roughly 200 lines of C, prebuilt for win32-x64 and win32-arm64, shipped as platform packages Yes Most auditable. Needs a Windows build job (run outside the credentialed jobs, under the pinning rules in AGENTS.md), pack:verify on Windows, prebuilds per supported Node ABI, and RELEASING.md changes.

Recommendation

  • Track C1. Once node:ffi is stable and on by default, and engines can
    move to that Node version, it closes Windows with no native dependency.
  • If Windows cannot wait for that, choose between C2 (fast, but trusts a
    third-party binary) and C3 (owned, but costs release-pipeline work). That is
    a maintainer policy decision, not an engineering one.
  • Do not adopt B unless the random refusals are acceptable.

Acceptance

  • On Windows, swapping a directory on the path for a junction between
    validation and use is refused for write_file (create and overwrite),
    edit_file, create_directory and the move_file destination. Nothing
    lands outside, and no outside file is modified.
  • The regression tests stage the swap deterministically and run on the
    windows-latest leg of typescript.yml. They fail without the fix.
  • Junctions and symlinks that resolve inside the allowed directories
    keep working.
  • FAT/exFAT volumes and older Windows releases still work, through the
    flag-retry paths.
  • Linux and macOS behaviour is unchanged.
  • Per-file coverage of at least 90 on every dimension.
  • The advisory's text is updated to say Windows is fixed, with the version
    that fixes it.

References

Lingua principale
TypeScript
Stelle
91.7k
Fork
11.9k
Merge medio
3h 30m
PR unite (30g)
204

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di modelcontextprotocol/servers

Tutte le issue di modelcontextprotocol/servers

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.