dsh-host-directory-picker-native: fixed 32KB koffi.view() read of a small heap pointer segfaults the host on Windows #4330
Unanswered
zhanglunet
asked this question in
Q&A
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Summary
@deepseek-ai/dsh-host-directory-picker-native's Win32 folder-dialog backend reads a fixed32768-byte view from a small heap pointer to extract the result path. On a real Windows
machine this reliably segfaults the whole
dshhost process (not just the picker) everytime a user completes (or cancels-then-reopens) the native folder dialog.
Reproduced 6 times back-to-back in a real user's
dsh-host.log(two separate host restarts,identical stack both times):
Root cause
worker.cjs(compiled frompackages/host/directory-picker-native,win32-dialog-bindings.ts):addresscomes fromIShellItem::GetDisplayName(SIGDN_FILESYSPATH), aCoTaskMemAllocblock sized only for the path + NUL terminator — typically tens to a few hundred bytes, never
32KB.
koffi.view(address, 32768)followed byBuffer.from()immediately materializes all32768 bytes, walking well past the actual allocation. Whenever that small block happens to sit
near the end of a committed memory page (which is common — it depends on nothing the caller
controls), the read crosses into unmapped memory and the process takes an access violation.
Node surfaces this as the confusing
FATAL ERROR: Error::New napi_get_last_error_infomessagebecause the N-API environment is already corrupted by the time it tries to construct the error.
The neighboring comment explains this 32KB-view approach was chosen specifically to avoid a
different crash from calling
koffi.decode(addr, 'str16')directly (that one apparentlydouble-dereferences the
_Out_ void **out-param). It looks like that fix traded one crash foranother — the new one is more reliably reproducible than the one it replaced, since virtually
every real folder-picker interaction produces a small allocation that's a strong candidate for
page-boundary proximity.
Not fixed in the latest published version
Confirmed the current version and the newest one (
npm view @deepseek-ai/dsh-host-directory-picker-native versions→ latest is
0.1.1-rc.2) have byte-identicalreadUtf16. This isn't something we can routearound by bumping the pinned dependency version.
Suggested direction
Read in small growing chunks (e.g. start at 520 bytes ≈
MAX_PATHUTF-16, double up to the32768 cap only if no NUL terminator is found) instead of materializing the full 32768-byte view
in one shot. That keeps the overwhelmingly common case (paths under
MAX_PATH) to a singlesmall read, which is far less likely to cross into an unmapped page. It doesn't eliminate the
theoretical risk entirely (no bound is provably safe without knowing the real allocation size),
but it should turn "crashes basically every time" into "crashes basically never."
Environment
@deepseek-ai/dsh-host-directory-picker-native@0.1.0-rc.7(pinned by our app; also checked0.1.1-rc.2, same code)dsh-host.logif useful.Per CONTRIBUTING.md, posting here as a Discussion rather than an Issue since Issues are
disabled on this repo — let me know if there's a better place for this.
All reactions