Skip to content

Commit

Permalink
fs,permission: make handling of buffers consistent
Browse files Browse the repository at this point in the history
Commit 2000c26 added explicit handling
of Buffers to fs.symlink, but not to fs.symlinkSync or
fs.promises.symlink. This change adapts the latter two functions to
behave like fs.symlink.

Refs: #49156
Refs: #51212
PR-URL: #52348
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
  • Loading branch information
tniessen authored Apr 6, 2024
1 parent 3f5ff8d commit 8c0b723
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
6 changes: 5 additions & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,11 @@ function symlinkSync(target, path, type) {
if (permission.isEnabled()) {
// The permission model's security guarantees fall apart in the presence of
// relative symbolic links. Thus, we have to prevent their creation.
if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
if (BufferIsBuffer(target)) {
if (!isAbsolute(BufferToString(target))) {
throw new ERR_ACCESS_DENIED('relative symbolic link target');
}
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
throw new ERR_ACCESS_DENIED('relative symbolic link target');
}
}
Expand Down
9 changes: 8 additions & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const {
SymbolAsyncDispose,
Uint8Array,
FunctionPrototypeBind,
uncurryThis,
} = primordials;

const { fs: constants } = internalBinding('constants');
Expand All @@ -30,6 +31,8 @@ const {

const binding = internalBinding('fs');
const { Buffer } = require('buffer');
const { isBuffer: BufferIsBuffer } = Buffer;
const BufferToString = uncurryThis(Buffer.prototype.toString);

const {
codes: {
Expand Down Expand Up @@ -985,7 +988,11 @@ async function symlink(target, path, type_) {
if (permission.isEnabled()) {
// The permission model's security guarantees fall apart in the presence of
// relative symbolic links. Thus, we have to prevent their creation.
if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
if (BufferIsBuffer(target)) {
if (!isAbsolute(BufferToString(target))) {
throw new ERR_ACCESS_DENIED('relative symbolic link target');
}
} else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) {
throw new ERR_ACCESS_DENIED('relative symbolic link target');
}
}
Expand Down

0 comments on commit 8c0b723

Please sign in to comment.