Skip to content

Commit

Permalink
permission: handle fs path traversal
Browse files Browse the repository at this point in the history
  • Loading branch information
RafaelGSS committed Jun 20, 2023
1 parent e15cc45 commit 205f1e6
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 6 deletions.
2 changes: 1 addition & 1 deletion lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
6 changes: 2 additions & 4 deletions lib/internal/process/permission.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
}

Expand Down
47 changes: 47 additions & 0 deletions test/fixtures/permission/fs-traversal.js
Original file line number Diff line number Diff line change
@@ -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));
}
3 changes: 2 additions & 1 deletion test/parallel/test-cli-permission-deny-fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
51 changes: 51 additions & 0 deletions test/parallel/test-permission-fs-traversal-path.js
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit 205f1e6

Please sign in to comment.