fs,permission: make handling of buffers consistent

tniessen · tniessen · commit f9a5145ac5fb · 2024-04-03T15:32:13.000+01:00
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: nodejs#49156 Refs: nodejs#51212
diff --git a/lib/fs.js b/lib/fs.js
@@ -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');
     }
   }
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -17,6 +17,7 @@ const {
   SymbolAsyncDispose,
   Uint8Array,
   FunctionPrototypeBind,
+  uncurryThis,
 } = primordials;
 
 const { fs: constants } = internalBinding('constants');
@@ -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: {
@@ -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');
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -1793,7 +1793,11 @@ function symlinkSync(target, path, type) {`
`1793`	`1793`	`if (permission.isEnabled()) {`
`1794`	`1794`	`// The permission model's security guarantees fall apart in the presence of`
`1795`	`1795`	`// relative symbolic links. Thus, we have to prevent their creation.`
`1796`		`- if (typeof target !== 'string' \|\| !isAbsolute(toPathIfFileURL(target))) {`
	`1796`	`+ if (BufferIsBuffer(target)) {`
	`1797`	`+ if (!isAbsolute(BufferToString(target))) {`
	`1798`	`+ throw new ERR_ACCESS_DENIED('relative symbolic link target');`
	`1799`	`+ }`
	`1800`	`+ } else if (typeof target !== 'string' \|\| !isAbsolute(toPathIfFileURL(target))) {`
`1797`	`1801`	`throw new ERR_ACCESS_DENIED('relative symbolic link target');`
`1798`	`1802`	`}`
`1799`	`1803`	`}`