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 <rafael.nunu@hotmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
  • Loading branch information
tniessen authored and marco-ippolito committed May 2, 2024
1 parent 69ebf18 commit c0010ed
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 @@ -1791,7 +1791,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 @@ -18,6 +18,7 @@ const {
SymbolAsyncDispose,
Uint8Array,
FunctionPrototypeBind,
uncurryThis,
} = primordials;

const { fs: constants } = internalBinding('constants');
Expand All @@ -31,6 +32,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 @@ -980,7 +983,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 c0010ed

Please sign in to comment.