Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node should log denied path by permission restriction #49080

Closed
KennyLindahl opened this issue Aug 9, 2023 · 8 comments
Closed

Node should log denied path by permission restriction #49080

KennyLindahl opened this issue Aug 9, 2023 · 8 comments
Labels
permission Issues and PRs related to the Permission Model question Issues that look for answers.

Comments

@KennyLindahl
Copy link

KennyLindahl commented Aug 9, 2023

What is the problem this feature will solve?

Description

Running the following:

$ node --experimental-permission index.js
// index.js
console.log("Hello, world!")

Generates:

node:internal/modules/cjs/loader:178
  const result = internalModuleStat(filename);
                 ^

Error: Access to this API has been restricted
    at stat (node:internal/modules/cjs/loader:178:18)
    at Module._findPath (node:internal/modules/cjs/loader:629:16)
    at resolveMainPath (node:internal/modules/run_main:15:25)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:24)
    at node:internal/main/run_main_module:23:47 {
  code: 'ERR_ACCESS_DENIED',
  permission: 'FileSystemRead',
  resource: '/Users/kenny.lindahl/Dev/test/node-20-permissions/index.js'
}

The problem

The problem with the above error is that the only way for me to make the error go away is to add --allow-fs-read which i don't want to do as it would make my app more vulnerable.

The solution

The solution is to print what path that was denied so that i can explicitly allow that path


  • Node version: v20.5.0

What is the feature you are proposing to solve the problem?

The same command should generate (similar to Deno):

$ node --experimental-permission index.js
Error: Access to this API has been restricted
 {
  code: 'ERR_ACCESS_DENIED',
  permission: 'FileSystemRead',
  resource: '/Users/kenny.lindahl/Dev/test/node-20-permissions/index.js',
  details: 'Read access was denied for: /the-path-that-was-denied'
}

Note: Observe the added details key, which highlights the path that was denied.
And this is needed for me to explicitly allow that path.

What alternatives have you considered?

No response

@KennyLindahl KennyLindahl added the feature request Issues that request new features to be added to Node.js. label Aug 9, 2023
@github-project-automation github-project-automation bot moved this to Pending Triage in Node.js feature requests Aug 9, 2023
@targos
Copy link
Member

targos commented Aug 9, 2023

resource: '/Users/kenny.lindahl/Dev/test/node-20-permissions/index.js'

This is the path that was denied.

@KennyLindahl
Copy link
Author

That's what i thought first, but then something like this should work (which it doesn't):

node --experimental-permission --allow-fs-read=./index.js index.js
node --experimental-permission --allow-fs-read=index.js index.js

Response:

$ node --experimental-permission --allow-fs-read=./index.js index.js
node:internal/modules/cjs/loader:178
  const result = internalModuleStat(filename);
                 ^

Error: Access to this API has been restricted
    at stat (node:internal/modules/cjs/loader:178:18)
    at Module._findPath (node:internal/modules/cjs/loader:629:16)
    at resolveMainPath (node:internal/modules/run_main:15:25)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:76:24)
    at node:internal/main/run_main_module:23:47 {
  code: 'ERR_ACCESS_DENIED',
  permission: 'FileSystemRead',
  resource: '/Users/kenny.lindahl/Dev/test/node-20-permissions/index.js'
}

@targos
Copy link
Member

targos commented Aug 9, 2023

https://nodejs.org/dist/latest-v20.x/docs/api/permissions.html#limitations-and-known-issues

Relative paths are not supported through the CLI (--allow-fs-*).

On Unix you could do:

node --experimental-permission --allow-fs-read=$(realpath index.js) index.js

@targos targos added question Issues that look for answers. and removed feature request Issues that request new features to be added to Node.js. labels Aug 9, 2023
@KennyLindahl
Copy link
Author

@targos Thanks, i will try it out tomorrow.
When do you think relative paths will be available?
Because without relative paths it will be difficult to make the permissions work without change on different computers (devs/server etc).

@targos
Copy link
Member

targos commented Aug 10, 2023

/cc @RafaelGSS

Btw, it seems we have no label for permission-related issues, nor a GH team.

@KennyLindahl
Copy link
Author

I got it to work fine with the following:

node --experimental-permission --allow-fs-read=$(realpath ./index.js),$(realpath ./node_modules) index.js

The program i'm running is using a fake malicious NPM package, which still could:

  • Get the victims IP and send it to the attacker (this is expected since there is no URL whitelist)

So my question is if there will be HTTP request block + whitelisting of URLs?

My fake malicious NPM package tried to start a local tunnel via spawn, and this was blocked which is good.
However this was never logged in in the Node program, it was only captured and delivered as text to the attackers computer (see code below).
Is this expected? In a real scenario i would never know what the attacker has tried to do etc, which is not ideal.

This is what the code looked like in the fake malicious NPM package

function spawnHelper(command, args, onData) {
  return new Promise((resolve, reject) => {
    try {
      const commandResponse = spawn(command, args);

      commandResponse.stdout.on("data", (data) => {
        onData(data.toString());
      });

      commandResponse.stderr.on("data", (data) => {
        onData(data.toString());
      });

      commandResponse.on("error", (error) => {
        onData(error.toString());
        reject(error);
      });
    } catch (error) {
      onData(error.toString());
    }
  });
}

And executed via:

await spawnHelper(localTunnelBinPath, ["--port", PORT], report);

Thanks

@targos
Copy link
Member

targos commented Aug 10, 2023

Those are good points, and make me think about other features that could be useful:

  • Flag to make Node.js abort (instead of throw) when an action is blocked
  • Side-channel to notify blocks (could be part of the diagnostics_channel module)

@RafaelGSS RafaelGSS added the permission Issues and PRs related to the Permission Model label Aug 10, 2023
@RafaelGSS
Copy link
Member

@targos Thanks, i will try it out tomorrow. When do you think relative paths will be available? Because without relative paths it will be difficult to make the permissions work without change on different computers (devs/server etc).

I'm working on it. For reference see: nodejs/security-wg#898.

My fake malicious NPM package tried to start a local tunnel via spawn, and this was blocked which is good.
However this was never logged in in the Node program, it was only captured and delivered as text to the attackers computer (see code below).
Is this expected? In a real scenario i would never know what the attacker has tried to do etc, which is not ideal.

Those are definitely good points, would you mind sharing them in the issue above so we can discuss and find a good approach to make it happen?

Anyway, I'm closing this issue since the initial question was addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
permission Issues and PRs related to the Permission Model question Issues that look for answers.
Projects
None yet
Development

No branches or pull requests

3 participants