diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 03fd6c7aee861b..763f39af62bbbb 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -709,7 +709,7 @@ const validatePath = hideStackFrames((path, propName = 'path') => { // The permission model needs the absolute path for the fs_permission function possiblyTransformPath(path) { if (permission.isEnabled()) { - if (typeof path === 'string' && !pathModule.isAbsolute(path)) { + if (typeof path === 'string') { return pathModule.resolve(path); } } diff --git a/lib/internal/process/permission.js b/lib/internal/process/permission.js index e400dcae9f934e..f17bfade519bfb 100644 --- a/lib/internal/process/permission.js +++ b/lib/internal/process/permission.js @@ -7,7 +7,7 @@ const { const permission = internalBinding('permission'); const { validateString } = require('internal/validators'); -const { isAbsolute, resolve } = require('path'); +const { resolve } = require('path'); let experimentalPermission; @@ -26,9 +26,7 @@ module.exports = ObjectFreeze({ // TODO: add support for WHATWG URLs and Uint8Arrays. validateString(reference, 'reference'); if (StringPrototypeStartsWith(scope, 'fs')) { - if (!isAbsolute(reference)) { - reference = resolve(reference); - } + reference = resolve(reference); } } diff --git a/test/fixtures/permission/fs-traversal.js b/test/fixtures/permission/fs-traversal.js new file mode 100644 index 00000000000000..7bf1d523dad674 --- /dev/null +++ b/test/fixtures/permission/fs-traversal.js @@ -0,0 +1,47 @@ +'use strict' + +const common = require('../../common'); + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const blockedFolder = process.env.BLOCKEDFOLDER; +const allowedFolder = process.env.ALLOWEDFOLDER; +const traversalPath = allowedFolder + '../file.md' + +{ + assert.ok(process.permission.has('fs.read', allowedFolder)); + assert.ok(process.permission.has('fs.write', allowedFolder)); + assert.ok(!process.permission.has('fs.read', blockedFolder)); + assert.ok(!process.permission.has('fs.write', blockedFolder)); +} + +{ + assert.throws(() => { + fs.writeFile(traversalPath, 'test', (error) => { + assert.ifError(error); + }); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.resolve(traversalPath)), + })); +} + +{ + assert.throws(() => { + fs.readFile(traversalPath, (error) => { + assert.ifError(error); + }); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(path.resolve(traversalPath)), + })); +} + +{ + assert.ok(!process.permission.has('fs.read', traversalPath)); + assert.ok(!process.permission.has('fs.write', traversalPath)); +} \ No newline at end of file diff --git a/test/parallel/test-cli-permission-deny-fs.js b/test/parallel/test-cli-permission-deny-fs.js index 927d582094cd41..964a0ad0a0e0c2 100644 --- a/test/parallel/test-cli-permission-deny-fs.js +++ b/test/parallel/test-cli-permission-deny-fs.js @@ -27,11 +27,12 @@ const path = require('path'); } { + const tmpPath = path.resolve('/tmp/'); const { status, stdout } = spawnSync( process.execPath, [ '--experimental-permission', - '--allow-fs-write', '/tmp/', '-e', + '--allow-fs-write', tmpPath, '-e', `console.log(process.permission.has("fs")); console.log(process.permission.has("fs.read")); console.log(process.permission.has("fs.write")); diff --git a/test/parallel/test-permission-fs-traversal-path.js b/test/parallel/test-permission-fs-traversal-path.js new file mode 100644 index 00000000000000..2b294a4574e2f3 --- /dev/null +++ b/test/parallel/test-permission-fs-traversal-path.js @@ -0,0 +1,51 @@ +// Flags: --experimental-permission --allow-fs-read=* --allow-fs-write=* --allow-child-process +'use strict'; + +const common = require('../common'); +common.skipIfWorker(); + +const fixtures = require('../common/fixtures'); +if (!common.canCreateSymLink()) + common.skip('insufficient privileges'); +if (!common.hasCrypto) + common.skip('no crypto'); + +const assert = require('assert'); +const fs = require('fs'); +const { spawnSync } = require('child_process'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); + +const file = fixtures.path('permission', 'fs-traversal.js'); +const blockedFolder = tmpdir.path; +const allowedFolder = path.join(tmpdir.path, 'subdirectory/'); +const commonPathWildcard = path.join(__filename, '../../common*'); + +{ + tmpdir.refresh(); + fs.mkdirSync(allowedFolder); +} + +{ + const { status, stderr } = spawnSync( + process.execPath, + [ + '--experimental-permission', + `--allow-fs-read=${file},${commonPathWildcard},${allowedFolder}`, + `--allow-fs-write=${allowedFolder}`, + file, + ], + { + env: { + ...process.env, + BLOCKEDFOLDER: blockedFolder, + ALLOWEDFOLDER: allowedFolder, + }, + } + ); + assert.strictEqual(status, 0, stderr.toString()); +} + +{ + tmpdir.refresh(); +}