From f4987eb91ec4256451a129f23d9795315ceea349 Mon Sep 17 00:00:00 2001 From: Rafael Gonzaga Date: Thu, 21 Dec 2023 17:36:13 +0000 Subject: [PATCH] lib,permission: handle buffer on fs.symlink PR-URL: https://github.com/nodejs/node/pull/51212 Reviewed-By: Stephen Belanger Reviewed-By: Paolo Insogna --- lib/fs.js | 10 +++++++++- .../test-permission-fs-symlink-relative.js | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index b2d00de6637304..b0781293bbcf96 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -40,6 +40,7 @@ const { StringPrototypeCharCodeAt, StringPrototypeIndexOf, StringPrototypeSlice, + uncurryThis, } = primordials; const { fs: constants } = internalBinding('constants'); @@ -66,6 +67,8 @@ const binding = internalBinding('fs'); const { createBlobFromFilePath } = require('internal/blob'); const { Buffer } = require('buffer'); +const { isBuffer: BufferIsBuffer } = Buffer; +const BufferToString = uncurryThis(Buffer.prototype.toString); const { aggregateTwoErrors, codes: { @@ -1721,7 +1724,12 @@ function symlink(target, path, type_, callback_) { 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))) { + callback(new ERR_ACCESS_DENIED('relative symbolic link target')); + return; + } + } else if (typeof target !== 'string' || !isAbsolute(toPathIfFileURL(target))) { callback(new ERR_ACCESS_DENIED('relative symbolic link target')); return; } diff --git a/test/parallel/test-permission-fs-symlink-relative.js b/test/parallel/test-permission-fs-symlink-relative.js index e7cd99ad5f227a..4cc7d920593c23 100644 --- a/test/parallel/test-permission-fs-symlink-relative.js +++ b/test/parallel/test-permission-fs-symlink-relative.js @@ -5,6 +5,7 @@ const common = require('../common'); common.skipIfWorker(); const assert = require('assert'); +const path = require('path'); const { symlinkSync, symlink, promises: { symlink: symlinkAsync } } = require('fs'); const error = { @@ -25,3 +26,16 @@ for (const targetString of ['a', './b/c', '../d', 'e/../f', 'C:drive-relative', } } } + +// Absolute should not throw +for (const targetString of [path.resolve('.')]) { + for (const target of [targetString, Buffer.from(targetString)]) { + for (const path of [__filename]) { + symlink(target, path, common.mustCall((err) => { + assert(err); + assert.strictEqual(err.code, 'EEXIST'); + assert.match(err.message, /file already exists/); + })); + } + } +}