From 9a9f538f9f35d3f511ea47900eef2d46d920bd26 Mon Sep 17 00:00:00 2001 From: Marcos Candeia Date: Mon, 11 Nov 2024 07:36:15 -0300 Subject: [PATCH 1/4] Fix access check Signed-off-by: Marcos Candeia --- src/stats.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/stats.ts b/src/stats.ts index c0f52678..98a35539 100644 --- a/src/stats.ts +++ b/src/stats.ts @@ -221,14 +221,24 @@ export abstract class StatsCommon implements Node.Sta * @internal */ public hasAccess(mode: number): boolean { + // Assuming 'credentials' and 'this.uid', 'this.gid', 'this.mode' are accessible if (credentials.euid === 0 || credentials.egid === 0) { - //Running as root + // Running as root return true; } - // Mask for - const adjusted = (credentials.uid == this.uid ? S_IRWXU : 0) | (credentials.gid == this.gid ? S_IRWXG : 0) | S_IRWXO; - return (mode & this.mode & adjusted) == mode; + // Build the adjusted permission mask based on ownership + let adjusted = 0; + if (credentials.uid === this.uid) { + adjusted |= S_IRWXU; // Include owner permissions + } + if (credentials.gid === this.gid) { + adjusted |= S_IRWXG; // Include group permissions + } + adjusted |= S_IRWXO; // Always include others' permissions + + // Perform the access check + return (this.mode & adjusted & mode) === mode; } /** From f32e9095d202607a6507f9c675de95efe5de2ef9 Mon Sep 17 00:00:00 2001 From: Marcos Candeia Date: Mon, 11 Nov 2024 10:30:03 -0300 Subject: [PATCH 2/4] Fixes GID set and hasAccess func Signed-off-by: Marcos Candeia --- src/file.ts | 2 +- src/inode.ts | 6 ++--- src/stats.ts | 52 ++++++++++++++++++++++++++++++++++--------- tests/fs/stat.test.ts | 42 ++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 15 deletions(-) diff --git a/src/file.ts b/src/file.ts index b3424695..1e00b64a 100644 --- a/src/file.ts +++ b/src/file.ts @@ -1,6 +1,6 @@ import type { FileReadResult } from 'node:fs/promises'; -import { O_APPEND, O_CREAT, O_EXCL, O_RDONLY, O_RDWR, O_SYNC, O_TRUNC, O_WRONLY, S_IFMT, size_max } from './emulation/constants.js'; import { config } from './emulation/config.js'; +import { O_APPEND, O_CREAT, O_EXCL, O_RDONLY, O_RDWR, O_SYNC, O_TRUNC, O_WRONLY, S_IFMT, size_max } from './emulation/constants.js'; import { Errno, ErrnoError } from './error.js'; import type { FileSystem } from './filesystem.js'; import './polyfills.js'; diff --git a/src/inode.ts b/src/inode.ts index 9449d425..933d331f 100644 --- a/src/inode.ts +++ b/src/inode.ts @@ -1,5 +1,5 @@ +import { deserialize, serialize, sizeof, struct, types as t } from 'utilium'; import { Stats, type StatsLike } from './stats.js'; -import { types as t, struct, sizeof, serialize, deserialize } from 'utilium'; /** * Alias for an ino. @@ -108,8 +108,8 @@ export class Inode implements StatsLike { hasChanged = true; } - if (this.uid !== stats.uid) { - this.uid = stats.uid; + if (this.gid !== stats.gid) { + this.gid = stats.gid; hasChanged = true; } diff --git a/src/stats.ts b/src/stats.ts index 98a35539..b6a9e5e5 100644 --- a/src/stats.ts +++ b/src/stats.ts @@ -1,6 +1,28 @@ import type * as Node from 'node:fs'; import { credentials, type Credentials } from './credentials.js'; -import { S_IFBLK, S_IFCHR, S_IFDIR, S_IFIFO, S_IFLNK, S_IFMT, S_IFREG, S_IFSOCK, S_IRWXG, S_IRWXO, S_IRWXU, size_max } from './emulation/constants.js'; +import { + R_OK, + S_IFBLK, + S_IFCHR, + S_IFDIR, + S_IFIFO, + S_IFLNK, + S_IFMT, + S_IFREG, + S_IFSOCK, + S_IRGRP, + S_IROTH, + S_IRUSR, + S_IWGRP, + S_IWOTH, + S_IWUSR, + S_IXGRP, + S_IXOTH, + S_IXUSR, + size_max, + W_OK, + X_OK, +} from './emulation/constants.js'; /** * Indicates the type of a file. Applied to 'mode'. @@ -221,24 +243,32 @@ export abstract class StatsCommon implements Node.Sta * @internal */ public hasAccess(mode: number): boolean { - // Assuming 'credentials' and 'this.uid', 'this.gid', 'this.mode' are accessible if (credentials.euid === 0 || credentials.egid === 0) { - // Running as root + //Running as root return true; } - // Build the adjusted permission mask based on ownership - let adjusted = 0; + let perm = 0; + if (credentials.uid === this.uid) { - adjusted |= S_IRWXU; // Include owner permissions - } - if (credentials.gid === this.gid) { - adjusted |= S_IRWXG; // Include group permissions + // Owner permissions + if (this.mode & S_IRUSR) perm |= R_OK; + if (this.mode & S_IWUSR) perm |= W_OK; + if (this.mode & S_IXUSR) perm |= X_OK; + } else if (credentials.gid === this.gid) { + // Group permissions + if (this.mode & S_IRGRP) perm |= R_OK; + if (this.mode & S_IWGRP) perm |= W_OK; + if (this.mode & S_IXGRP) perm |= X_OK; + } else { + // Others permissions + if (this.mode & S_IROTH) perm |= R_OK; + if (this.mode & S_IWOTH) perm |= W_OK; + if (this.mode & S_IXOTH) perm |= X_OK; } - adjusted |= S_IRWXO; // Always include others' permissions // Perform the access check - return (this.mode & adjusted & mode) === mode; + return (perm & mode) === mode; } /** diff --git a/tests/fs/stat.test.ts b/tests/fs/stat.test.ts index 344fd666..10211a17 100644 --- a/tests/fs/stat.test.ts +++ b/tests/fs/stat.test.ts @@ -1,5 +1,6 @@ import assert from 'node:assert'; import { suite, test } from 'node:test'; +import { credentials } from '../../dist/credentials.js'; import { Stats } from '../../dist/stats.js'; import { fs } from '../common.js'; @@ -34,6 +35,47 @@ suite('Stats', () => { fs.close(fd); }); + test('hasAccess for non-root access', () => { + const newFile = 'new.txt'; + + fs.writeFileSync(newFile, 'hello', { + mode: 0o640, // allow group access + }); + + const prevCredentials = { + ...credentials, + }; + const uid = 33; + const nonRootCredentials = { + uid, + gid: uid, + euid: uid, + egid: uid, + suid: uid, + sgid: uid, + }; + + fs.chownSync(newFile, 0, nonRootCredentials.gid); // creating with root-user so that non-root user can access + + Object.assign(credentials, nonRootCredentials); + const stat = fs.statSync(newFile); + + assert.equal(stat.gid, nonRootCredentials.gid); + assert.equal(stat.uid, 0); + assert.equal(stat.hasAccess(fs.constants.R_OK), true); + assert.equal(stat.hasAccess(fs.constants.W_OK), false); + assert.equal(stat.hasAccess(fs.constants.X_OK), false); + // changing group + + Object.assign(credentials, { ...nonRootCredentials, gid: 44 }); + + assert.equal(stat.hasAccess(fs.constants.R_OK), false); + assert.equal(stat.hasAccess(fs.constants.W_OK), false); + assert.equal(stat.hasAccess(fs.constants.X_OK), false); + + Object.assign(credentials, prevCredentials); + }); + test('stat file', async () => { const stats = await fs.promises.stat(existing_file); assert(!stats.isDirectory()); From 3dfa97f61732df59996913b52b469f4c52bd5e0c Mon Sep 17 00:00:00 2001 From: Marcos Candeia Date: Mon, 11 Nov 2024 14:26:07 -0300 Subject: [PATCH 3/4] Fixes conditional logic based on if/else Signed-off-by: Marcos Candeia --- src/stats.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/stats.ts b/src/stats.ts index b6a9e5e5..30b032c8 100644 --- a/src/stats.ts +++ b/src/stats.ts @@ -244,24 +244,28 @@ export abstract class StatsCommon implements Node.Sta */ public hasAccess(mode: number): boolean { if (credentials.euid === 0 || credentials.egid === 0) { - //Running as root + // Running as root return true; } let perm = 0; + // Owner permissions if (credentials.uid === this.uid) { - // Owner permissions if (this.mode & S_IRUSR) perm |= R_OK; if (this.mode & S_IWUSR) perm |= W_OK; if (this.mode & S_IXUSR) perm |= X_OK; - } else if (credentials.gid === this.gid) { - // Group permissions + } + + // Group permissions + if (credentials.gid === this.gid) { if (this.mode & S_IRGRP) perm |= R_OK; if (this.mode & S_IWGRP) perm |= W_OK; if (this.mode & S_IXGRP) perm |= X_OK; - } else { - // Others permissions + } + + // Others permissions + if (credentials.uid !== this.uid && credentials.gid !== this.gid) { if (this.mode & S_IROTH) perm |= R_OK; if (this.mode & S_IWOTH) perm |= W_OK; if (this.mode & S_IXOTH) perm |= X_OK; From 9dac60d95f2fb933f71e1941bcc30718fd3a3de9 Mon Sep 17 00:00:00 2001 From: James Prevett Date: Mon, 11 Nov 2024 11:32:33 -0600 Subject: [PATCH 4/4] Fixed `Stats.hasAccess` other permissions conditional --- src/stats.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/stats.ts b/src/stats.ts index 30b032c8..73bc8214 100644 --- a/src/stats.ts +++ b/src/stats.ts @@ -265,11 +265,9 @@ export abstract class StatsCommon implements Node.Sta } // Others permissions - if (credentials.uid !== this.uid && credentials.gid !== this.gid) { - if (this.mode & S_IROTH) perm |= R_OK; - if (this.mode & S_IWOTH) perm |= W_OK; - if (this.mode & S_IXOTH) perm |= X_OK; - } + if (this.mode & S_IROTH) perm |= R_OK; + if (this.mode & S_IWOTH) perm |= W_OK; + if (this.mode & S_IXOTH) perm |= X_OK; // Perform the access check return (perm & mode) === mode;