permission: fix chmod,chown improve fs coverage

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/47529
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
pull/47542/head
Rafael Gonzaga 2023-04-13 11:06:44 -03:00 committed by GitHub
parent 6fb74c743c
commit 1323992672
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 176 additions and 16 deletions

View File

@ -1338,8 +1338,18 @@ static void Link(const FunctionCallbackInfo<Value>& args) {
BufferValue src(isolate, args[0]);
CHECK_NOT_NULL(*src);
const auto src_view = src.ToStringView();
// To avoid bypass the link target should be allowed to read and write
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemRead, src_view);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, src_view);
BufferValue dest(isolate, args[1]);
CHECK_NOT_NULL(*dest);
const auto dest_view = dest.ToStringView();
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, dest_view);
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
if (req_wrap_async != nullptr) { // link(src, dest, req)
@ -2422,6 +2432,8 @@ static void Chmod(const FunctionCallbackInfo<Value>& args) {
BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
CHECK(args[1]->IsInt32());
int mode = args[1].As<Int32>()->Value();
@ -2485,6 +2497,8 @@ static void Chown(const FunctionCallbackInfo<Value>& args) {
BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
CHECK(IsSafeJsInt(args[1]));
const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Integer>()->Value());
@ -2551,6 +2565,8 @@ static void LChown(const FunctionCallbackInfo<Value>& args) {
BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
CHECK(IsSafeJsInt(args[1]));
const uv_uid_t uid = static_cast<uv_uid_t>(args[1].As<Integer>()->Value());
@ -2646,6 +2662,8 @@ static void LUTimes(const FunctionCallbackInfo<Value>& args) {
BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
CHECK(args[1]->IsNumber());
const double atime = args[1].As<Number>()->Value();

View File

@ -5,14 +5,11 @@ const common = require('../../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const os = require('os');
const blockedFile = process.env.BLOCKEDFILE;
const blockedFolder = process.env.BLOCKEDFOLDER;
const allowedFolder = process.env.ALLOWEDFOLDER;
const regularFile = __filename;
const uid = os.userInfo().uid;
const gid = os.userInfo().gid;
// fs.readFile
{
@ -106,19 +103,6 @@ const gid = os.userInfo().gid;
});
}
// fs.chownSync (should not bypass)
{
assert.throws(() => {
// This operation will work fine
fs.chownSync(blockedFile, uid, gid);
fs.readFileSync(blockedFile);
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',
resource: path.toNamespacedPath(blockedFile),
}));
}
// fs.copyFile
{
assert.throws(() => {

View File

@ -31,6 +31,15 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
permission: 'FileSystemWrite',
resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')),
}));
assert.throws(() => {
fs.link(path.join(readOnlyFolder, 'file'), path.join(readWriteFolder, 'link-to-read-only'), (err) => {
assert.ifError(err);
});
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
resource: path.toNamespacedPath(path.join(readOnlyFolder, 'file')),
}));
// App will be able to symlink to a writeOnlyFolder
fs.symlink(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write'), 'file', (err) => {
@ -48,6 +57,21 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
// App will be able to write to the symlink
fs.writeFile(path.join(writeOnlyFolder, 'link-to-read-write'), 'some content', common.mustSucceed());
});
fs.link(path.join(readWriteFolder, 'file'), path.join(writeOnlyFolder, 'link-to-read-write2'), (err) => {
assert.ifError(err);
// App will won't be able to read the link
assert.throws(() => {
fs.readFile(path.join(writeOnlyFolder, 'link-to-read-write2'), (err) => {
assert.ifError(err);
});
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',
}));
// App will be able to write to the link
fs.writeFile(path.join(writeOnlyFolder, 'link-to-read-write2'), 'some content', common.mustSucceed());
});
// App won't be able to symlink to a readOnlyFolder
assert.throws(() => {
@ -59,4 +83,13 @@ const writeOnlyFolder = process.env.WRITEONLYFOLDER;
permission: 'FileSystemWrite',
resource: path.toNamespacedPath(path.join(readOnlyFolder, 'link-to-read-only')),
}));
assert.throws(() => {
fs.link(path.join(readWriteFolder, 'file'), path.join(readOnlyFolder, 'link-to-read-only'), (err) => {
assert.ifError(err);
});
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
resource: path.toNamespacedPath(path.join(readOnlyFolder, 'link-to-read-only')),
}));
}

View File

@ -80,6 +80,14 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
}));
assert.throws(() => {
fs.link(regularFile, blockedFolder + '/asdf', (err) => {
assert.ifError(err);
});
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
}));
// App won't be able to symlink BLOCKEDFILE to REGULARDIR
assert.throws(() => {
@ -90,4 +98,12 @@ const symlinkFromBlockedFile = process.env.EXISTINGSYMLINK;
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',
}));
assert.throws(() => {
fs.link(blockedFile, path.join(__dirname, '/asdf'), (err) => {
assert.ifError(err);
});
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemRead',
}));
}

View File

@ -108,6 +108,17 @@ const absoluteProtectedFolder = path.resolve(relativeProtectedFolder);
}));
}
// fs.lutimes
{
assert.throws(() => {
fs.lutimes(blockedFile, new Date(), new Date(), () => {});
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
resource: path.toNamespacedPath(blockedFile),
}));
}
// fs.mkdir
{
assert.throws(() => {
@ -270,3 +281,101 @@ const absoluteProtectedFolder = path.resolve(relativeProtectedFolder);
});
}
}
// fs.chmod
{
assert.throws(() => {
fs.chmod(blockedFile, 0o755, common.mustNotCall());
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
assert.rejects(async () => {
await fs.promises.chmod(blockedFile, 0o755);
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
}
// fs.lchmod
{
if (common.isOSX) {
assert.throws(() => {
fs.lchmod(blockedFile, 0o755, common.mustNotCall());
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
assert.rejects(async () => {
await fs.promises.lchmod(blockedFile, 0o755);
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
}
}
// fs.appendFile
{
assert.throws(() => {
fs.appendFile(blockedFile, 'new data', common.mustNotCall());
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
assert.rejects(async () => {
await fs.promises.appendFile(blockedFile, 'new data');
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
}
// fs.chown
{
assert.throws(() => {
fs.chown(blockedFile, 1541, 999, common.mustNotCall());
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
assert.rejects(async () => {
await fs.promises.chown(blockedFile, 1541, 999);
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
}
// fs.lchown
{
assert.throws(() => {
fs.lchown(blockedFile, 1541, 999, common.mustNotCall());
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
assert.rejects(async () => {
await fs.promises.lchown(blockedFile, 1541, 999);
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
}
// fs.link
{
assert.throws(() => {
fs.link(blockedFile, path.join(blockedFolder, '/linked'), common.mustNotCall());
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
assert.rejects(async () => {
await fs.promises.link(blockedFile, path.join(blockedFolder, '/linked'));
}, {
code: 'ERR_ACCESS_DENIED',
permission: 'FileSystemWrite',
});
}