Repository navigation
fix(ext/node): check permissions when binding node:net unix sockets - #35564
Merged
Merged
Conversation
The node-compat PipeWrap handle (the native backing for node:net
unix-domain sockets, also reachable via process.binding("pipe_wrap"))
bound, chmod'd, and unlinked filesystem paths without consulting Deno's
permission model. Binding a unix socket creates a socket inode on disk,
so bind() now requires read+write permission for the path, matching
connect() and Deno.listen({ transport: "unix" }). The permission check
lives at bind() rather than only at listen() because bind() is where the
filesystem mutation happens; a denied bind never records the path, so the
fchmod and unlink-on-close paths that operate on the bound path can no
longer target an arbitrary file.
Also record the bound path only after a successful bind(2), matching
libuv, so a failed bind can no longer arm fchmod/unlink against an
existing file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The node-compat
PipeWraphandle is the native backing fornode:netunix-domain sockets, and is also reachable directly from user code via
process.binding("pipe_wrap").Pipe. Several of its operations mutated thefilesystem without going through Deno's permission model.
Binding a unix-domain socket creates a socket inode on disk via
socket(2)+bind(2), yetbind()performed no permission check. Worse,uv_pipe_bindrecorded the bound path on the handle before attempting thebind and did not clear it on failure, so a failed bind against an existing
file still armed the two operations that act on the recorded path:
fchmod()(which calls
chmod(2)) and the unlink-on-close inclose_pipe(which callsremove_file). The net effect was that code granted no--allow-*flags couldcreate a socket inode at an arbitrary writable path, change the mode of an
existing owned file, or delete an arbitrary unlinkable file.
This moves the filesystem permission check to
bind(), where the inode isactually created, mirroring
connect()andDeno.listen({ transport: "unix" }).A denied bind throws
NotCapableand never records the path, so thefchmodand unlink-on-close paths can no longer reach an arbitrary file. As defense in
depth (and to match libuv, which stores the path on the handle only on success),
uv_pipe_bindnow records the bound path only afterbind(2)succeeds.The existing permission check in
listen()is left in place. A new spec testunder
tests/specs/permission/node_pipe_wrap_no_permsruns with no permissionsand asserts that
bind()is denied and that no path is left armed forfchmod.Legitimate unix-socket servers created with
--allow-read --allow-writecontinue to work, and the existing
pipe_open_fdandhttp_address_overrideunix-socket suites pass unchanged.