diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index c7ec755a306299..1d708d2722e76d 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -65,6 +65,8 @@ const kType = Symbol('type'); const kStats = Symbol('stats'); const assert = require('internal/assert'); +const { encodeUtf8String } = internalBinding('encoding_binding'); + const { fs: { F_OK = 0, @@ -719,7 +721,10 @@ function possiblyTransformPath(path) { } assert(isUint8Array(path)); if (!BufferIsBuffer(path)) path = BufferFrom(path); - return BufferFrom(resolvePath(BufferToString(path))); + // Avoid Buffer.from() and use a C++ binding instead to encode the result + // of path.resolve() in order to prevent path traversal attacks that + // monkey-patch Buffer internals. + return encodeUtf8String(resolvePath(BufferToString(path))); } return path; } diff --git a/test/fixtures/permission/fs-traversal.js b/test/fixtures/permission/fs-traversal.js index 87132177643c45..f0a14805aa9c71 100644 --- a/test/fixtures/permission/fs-traversal.js +++ b/test/fixtures/permission/fs-traversal.js @@ -96,6 +96,39 @@ const uint8ArrayTraversalPath = new TextEncoder().encode(traversalPath); })); } +// Monkey-patching Buffer internals should also not allow path traversal. +{ + const extraChars = '.'.repeat(40); + const traversalPathWithExtraChars = traversalPath + extraChars; + const traversalPathWithExtraBytes = Buffer.from(traversalPathWithExtraChars); + + Buffer.prototype.utf8Write = ((w) => function(str, ...args) { + assert.strictEqual(str, resolve(traversalPath) + extraChars); + return w.apply(this, [traversalPath, ...args]); + })(Buffer.prototype.utf8Write); + + // Sanity check (remove if the internals of Buffer.from change): + // The custom implementation of utf8Write should cause Buffer.from() to encode + // traversalPath instead of the sanitized output of resolve(). + assert.strictEqual(Buffer.from(resolve(traversalPathWithExtraChars)).toString(), traversalPath); + + assert.throws(() => { + fs.readFile(traversalPathWithExtraBytes, common.mustNotCall()); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: resolve(traversalPathWithExtraChars), + })); + + assert.throws(() => { + fs.readFile(new TextEncoder().encode(traversalPathWithExtraBytes.toString()), common.mustNotCall()); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: resolve(traversalPathWithExtraChars), + })); +} + { assert.ok(!process.permission.has('fs.read', traversalPath)); assert.ok(!process.permission.has('fs.write', traversalPath));