From 67ccde35261d464760f7e67313455459cf4db533 Mon Sep 17 00:00:00 2001 From: "Tristan F." Date: Thu, 27 Jun 2024 12:19:27 -0400 Subject: [PATCH 1/3] lib: add security warning on io full access --- doc/api/permissions.md | 6 ++++++ lib/internal/process/pre_execution.js | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/doc/api/permissions.md b/doc/api/permissions.md index d994035c808818..32499663528fae 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -584,6 +584,12 @@ There are constraints you need to know before using this system: * Using existing file descriptors via the `node:fs` module bypasses the Permission Model. +#### Allowing all write operations + +When `--allow-fs-write=*` is permitted, it may inadvertently lead to invalidating +the permission model because of unintended file access, like full write access +to memory with `/proc/self/mem`. + #### Limitations and Known Issues * Symbolic links will be followed even to locations outside of the set of paths diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 27df0a9440a03c..dc70034471e240 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -554,6 +554,22 @@ function initializePermission() { 'It could invalidate the permission model.', 'SecurityWarning'); } } + const fsReadValue = getOptionValue('--allow-fs-read'); + if (fsReadValue.length === 1 && (fsReadValue[0] === '*' || fsReadValue[0] === '/')) { + process.emitWarning( + 'Granting all to --allow-fs-read leaks all sensitive info on the host machine.', + 'SecurityWarning' + ); + } + const fsWriteValue = getOptionValue('--allow-fs-write'); + if (fsWriteValue.length === 1 && (fsWriteValue[0] === '*' || fsWriteValue[0] === '/')) { + process.emitWarning( + 'Granting all to --allow-fs-write will invalidate the permission model. ' + + 'Documentation can be found at ' + + 'https://nodejs.org/api/permissions.html#allowing-all-write-operations', + 'SecurityWarning' + ); + } const warnCommaFlags = [ '--allow-fs-read', '--allow-fs-write', From da71410bbb6864b3cf5e8894b707b1f1cb4af5c5 Mon Sep 17 00:00:00 2001 From: "Tristan F." Date: Sat, 29 Jun 2024 09:16:11 -0400 Subject: [PATCH 2/3] doc: more generic write operation warning --- doc/api/permissions.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/api/permissions.md b/doc/api/permissions.md index 32499663528fae..321e8d7cc01246 100644 --- a/doc/api/permissions.md +++ b/doc/api/permissions.md @@ -587,8 +587,10 @@ There are constraints you need to know before using this system: #### Allowing all write operations When `--allow-fs-write=*` is permitted, it may inadvertently lead to invalidating -the permission model because of unintended file access, like full write access -to memory with `/proc/self/mem`. +the permission model because of unintended file access +to files that have side effects when written to, like +service configuration files or internal file interfaces like +linux's `/proc`. #### Limitations and Known Issues From 4d24405f459104f6736acbbb6fbff2813aab2a27 Mon Sep 17 00:00:00 2001 From: "Tristan F." Date: Sat, 29 Jun 2024 09:16:19 -0400 Subject: [PATCH 3/3] lib: don't check for `/` on windows --- lib/internal/process/pre_execution.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index dc70034471e240..dec83d85308193 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -44,6 +44,8 @@ const { }, } = require('internal/v8/startup_snapshot'); +const isWindows = process.platform === 'win32'; + function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) { return prepareExecution({ expandArgv1, @@ -555,14 +557,14 @@ function initializePermission() { } } const fsReadValue = getOptionValue('--allow-fs-read'); - if (fsReadValue.length === 1 && (fsReadValue[0] === '*' || fsReadValue[0] === '/')) { + if (fsReadValue.length === 1 && (fsReadValue[0] === '*' || (!isWindows && fsReadValue[0] === '/'))) { process.emitWarning( 'Granting all to --allow-fs-read leaks all sensitive info on the host machine.', 'SecurityWarning' ); } const fsWriteValue = getOptionValue('--allow-fs-write'); - if (fsWriteValue.length === 1 && (fsWriteValue[0] === '*' || fsWriteValue[0] === '/')) { + if (fsWriteValue.length === 1 && (fsWriteValue[0] === '*' || (!isWindows && fsWriteValue[0] === '/'))) { process.emitWarning( 'Granting all to --allow-fs-write will invalidate the permission model. ' + 'Documentation can be found at ' +