diff --git a/src/node_file.cc b/src/node_file.cc index f5e7cc80e724..816318844614 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1407,17 +1407,10 @@ static void Symlink(const FunctionCallbackInfo& args) { BufferValue target(isolate, args[0]); CHECK_NOT_NULL(*target); auto target_view = target.ToStringView(); - // To avoid bypass the symlink target should be allowed to read and write - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemRead, target_view); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemWrite, target_view); BufferValue path(isolate, args[1]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); CHECK(args[2]->IsInt32()); int flags = args[2].As()->Value(); @@ -1425,6 +1418,22 @@ static void Symlink(const FunctionCallbackInfo& args) { if (argc > 3) { // symlink(target, path, flags, req) FSReqBase* req_wrap_async = GetReqWrap(args, 3); CHECK_NOT_NULL(req_wrap_async); + // To avoid bypass the symlink target should be allowed to read and write + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemRead, + target_view); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemWrite, + target_view); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN2(UV_FS_SYMLINK, req_wrap_async, "target", @@ -1444,6 +1453,15 @@ static void Symlink(const FunctionCallbackInfo& args) { *path, flags); } else { // symlink(target, path, flags, undefined, ctx) + // To avoid bypass the symlink target should be allowed to read and write + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemRead, target_view); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemWrite, target_view); + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FSReqWrapSync req_wrap_sync("symlink", *target, *path); FS_SYNC_TRACE_BEGIN(symlink); SyncCallAndThrowOnError( @@ -1534,14 +1552,17 @@ static void ReadLink(const FunctionCallbackInfo& args) { BufferValue path(isolate, args[0]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8); if (argc > 2) { // readlink(path, encoding, req) FSReqBase* req_wrap_async = GetReqWrap(args, 2); CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemRead, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN1( UV_FS_READLINK, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, @@ -1553,6 +1574,8 @@ static void ReadLink(const FunctionCallbackInfo& args) { uv_fs_readlink, *path); } else { // readlink(path, encoding) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, permission::PermissionScope::kFileSystemRead, path.ToStringView()); FSReqWrapSync req_wrap_sync("readlink", *path); FS_SYNC_TRACE_BEGIN(readlink); int err = @@ -1778,12 +1801,15 @@ static void RMDir(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); if (argc > 1) { FSReqBase* req_wrap_async = GetReqWrap(args, 1); // rmdir(path, req) CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN1( UV_FS_RMDIR, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, @@ -1795,6 +1821,10 @@ static void RMDir(const FunctionCallbackInfo& args) { uv_fs_rmdir, *path); } else { // rmdir(path) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FSReqWrapSync req_wrap_sync("rmdir", *path); FS_SYNC_TRACE_BEGIN(rmdir); SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_rmdir, *path); @@ -2156,9 +2186,6 @@ static void MKDir(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); - CHECK(args[1]->IsInt32()); const int mode = args[1].As()->Value(); @@ -2168,6 +2195,11 @@ static void MKDir(const FunctionCallbackInfo& args) { if (argc > 3) { // mkdir(path, mode, recursive, req) FSReqBase* req_wrap_async = GetReqWrap(args, 3); CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN1( UV_FS_UNLINK, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, @@ -2180,6 +2212,10 @@ static void MKDir(const FunctionCallbackInfo& args) { *path, mode); } else { // mkdir(path, mode, recursive) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FSReqWrapSync req_wrap_sync("mkdir", *path); FS_SYNC_TRACE_BEGIN(mkdir); if (mkdirp) { @@ -4117,8 +4153,6 @@ static void Chmod(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); CHECK(args[1]->IsInt32()); int mode = args[1].As()->Value(); @@ -4126,6 +4160,11 @@ static void Chmod(const FunctionCallbackInfo& args) { if (argc > 2) { // chmod(path, mode, req) FSReqBase* req_wrap_async = GetReqWrap(args, 2); CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN1( UV_FS_CHMOD, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, @@ -4138,6 +4177,10 @@ static void Chmod(const FunctionCallbackInfo& args) { *path, mode); } else { // chmod(path, mode) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FSReqWrapSync req_wrap_sync("chmod", *path); FS_SYNC_TRACE_BEGIN(chmod); SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_chmod, *path, mode); @@ -4333,8 +4376,6 @@ static void UTimes(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); CHECK(args[1]->IsNumber()); const double atime = args[1].As()->Value(); @@ -4345,6 +4386,11 @@ static void UTimes(const FunctionCallbackInfo& args) { if (argc > 3) { // utimes(path, atime, mtime, req) FSReqBase* req_wrap_async = GetReqWrap(args, 3); CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN1( UV_FS_UTIME, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, @@ -4358,6 +4404,10 @@ static void UTimes(const FunctionCallbackInfo& args) { atime, mtime); } else { // utimes(path, atime, mtime) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FSReqWrapSync req_wrap_sync("utime", *path); FS_SYNC_TRACE_BEGIN(utimes); SyncCallAndThrowOnError( @@ -4415,8 +4465,6 @@ static void LUTimes(const FunctionCallbackInfo& args) { BufferValue path(env->isolate(), args[0]); CHECK_NOT_NULL(*path); ToNamespacedPath(env, &path); - THROW_IF_INSUFFICIENT_PERMISSIONS( - env, permission::PermissionScope::kFileSystemWrite, path.ToStringView()); CHECK(args[1]->IsNumber()); const double atime = args[1].As()->Value(); @@ -4427,6 +4475,11 @@ static void LUTimes(const FunctionCallbackInfo& args) { if (argc > 3) { // lutimes(path, atime, mtime, req) FSReqBase* req_wrap_async = GetReqWrap(args, 3); CHECK_NOT_NULL(req_wrap_async); + ASYNC_THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + req_wrap_async, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FS_ASYNC_TRACE_BEGIN1( UV_FS_LUTIME, req_wrap_async, "path", TRACE_STR_COPY(*path)) AsyncCall(env, @@ -4440,6 +4493,10 @@ static void LUTimes(const FunctionCallbackInfo& args) { atime, mtime); } else { // lutimes(path, atime, mtime) + THROW_IF_INSUFFICIENT_PERMISSIONS( + env, + permission::PermissionScope::kFileSystemWrite, + path.ToStringView()); FSReqWrapSync req_wrap_sync("lutime", *path); FS_SYNC_TRACE_BEGIN(lutimes); SyncCallAndThrowOnError( diff --git a/test/fixtures/permission/fs-read.js b/test/fixtures/permission/fs-read.js index 2feb64d021b5..1446dd7bc579 100644 --- a/test/fixtures/permission/fs-read.js +++ b/test/fixtures/permission/fs-read.js @@ -721,6 +721,34 @@ const regularFile = __filename; }); } +// fs.readlink +{ + fs.readlink(blockedFile, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })); + fs.readlink(bufferBlockedFile, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })); + assert.throws(() => { + fs.readlinkSync(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })); + assert.rejects(async () => { + await fsPromises.readlink(blockedFile); + }, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemRead', + resource: path.toNamespacedPath(blockedFile), + })).then(common.mustCall()); +} + // fs.realpath (JS implementation, walks the path with lstat) { assert.throws(() => { diff --git a/test/fixtures/permission/fs-write.js b/test/fixtures/permission/fs-write.js index 7a9963e17d9e..4d226329e707 100644 --- a/test/fixtures/permission/fs-write.js +++ b/test/fixtures/permission/fs-write.js @@ -115,62 +115,59 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; // fs.utimes { - assert.throws(() => { - fs.utimes(blockedFile, new Date(), new Date(), () => {}); - }, { + fs.utimes(blockedFile, new Date(), new Date(), common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', resource: path.toNamespacedPath(blockedFile), - }); - assert.throws(() => { - fs.utimes(bufferBlockedFile, new Date(), new Date(), () => {}); - }, { + })); + fs.utimes(bufferBlockedFile, new Date(), new Date(), common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', resource: path.toNamespacedPath(blockedFile), - }); - assert.throws(() => { - fs.utimes(blockedFileURL, new Date(), new Date(), () => {}); - }, { + })); + fs.utimes(blockedFileURL, new Date(), new Date(), common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', resource: path.toNamespacedPath(blockedFile), - }); - assert.throws(() => { - fs.utimes(relativeProtectedFile, new Date(), new Date(), () => {}); - }, { + })); + fs.utimes(relativeProtectedFile, new Date(), new Date(), common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', resource: path.toNamespacedPath(relativeProtectedFile), - }); - + })); + fs.utimes(path.join(blockedFolder, 'anyfile'), new Date(), new Date(), common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + })); assert.throws(() => { - fs.utimes(path.join(blockedFolder, 'anyfile'), new Date(), new Date(), () => {}); + fs.utimesSync(blockedFile, new Date(), new Date()); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', - resource: path.toNamespacedPath(path.join(blockedFolder, 'anyfile')), + resource: path.toNamespacedPath(blockedFile), }); } // fs.lutimes { - assert.throws(() => { - fs.lutimes(blockedFile, new Date(), new Date(), () => {}); - },{ + fs.lutimes(blockedFile, new Date(), new Date(), common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', resource: path.toNamespacedPath(blockedFile), - }); - assert.throws(() => { - fs.lutimes(bufferBlockedFile, new Date(), new Date(), () => {}); - },{ + })); + fs.lutimes(bufferBlockedFile, new Date(), new Date(), common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', resource: path.toNamespacedPath(blockedFile), - }); + })); + fs.lutimes(blockedFileURL, new Date(), new Date(), common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFile), + })); assert.throws(() => { - fs.lutimes(blockedFileURL, new Date(), new Date(), () => {}); + fs.lutimesSync(blockedFile, new Date(), new Date()); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', @@ -180,24 +177,46 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; // fs.mkdir { + fs.mkdir(path.join(blockedFolder, 'any-folder'), common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(blockedFolder, 'any-folder')), + })); + fs.mkdir(path.join(relativeProtectedFolder, 'any-folder'), common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-folder')), + })); assert.throws(() => { - fs.mkdir(path.join(blockedFolder, 'any-folder'), (err) => { - assert.ifError(err); - }); - },{ + fs.mkdirSync(path.join(blockedFolder, 'any-folder')); + }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', resource: path.toNamespacedPath(path.join(blockedFolder, 'any-folder')), }); +} + +// fs.rmdir +{ + fs.rmdir(blockedFolder, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFolder), + })); assert.throws(() => { - fs.mkdir(path.join(relativeProtectedFolder, 'any-folder'), (err) => { - assert.ifError(err); - }); - },{ + fs.rmdirSync(blockedFolder); + }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', - resource: path.toNamespacedPath(path.join(relativeProtectedFolder, 'any-folder')), + resource: path.toNamespacedPath(blockedFolder), }); + assert.rejects(async () => { + await fsPromises.rmdir(blockedFolder); + }, { + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + resource: path.toNamespacedPath(blockedFolder), + }).then(common.mustCall()); } // fs.mkdtemp @@ -401,20 +420,20 @@ const relativeProtectedFolder = process.env.RELATIVEBLOCKEDFOLDER; // fs.chmod { - assert.throws(() => { - fs.chmod(blockedFile, 0o755, common.mustNotCall()); - }, { + fs.chmod(blockedFile, 0o755, common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', - }); - assert.throws(() => { - fs.chmod(bufferBlockedFile, 0o755, common.mustNotCall()); - }, { + })); + fs.chmod(bufferBlockedFile, 0o755, common.expectsError({ code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite', - }); + })); + fs.chmod(blockedFileURL, 0o755, common.expectsError({ + code: 'ERR_ACCESS_DENIED', + permission: 'FileSystemWrite', + })); assert.throws(() => { - fs.chmod(blockedFileURL, 0o755, common.mustNotCall()); + fs.chmodSync(blockedFile, 0o755); }, { code: 'ERR_ACCESS_DENIED', permission: 'FileSystemWrite',