security-wg icon indicating copy to clipboard operation
security-wg copied to clipboard

feature request for `require.pure(id)` or `pkg.pure:true`

Open LongTengDao opened this issue 6 years ago • 14 comments

Feature request.

Most third part module we are using are pure function things. If we can control that (when we require them even install them), then maybe most security problems could under control easily?

"pure module require" means:

  1. no i/o, like fs, net native module.
  2. no context pollute, like viciously rewritting to standard lib and native module exports.
  3. other things, like Buffer.allocUnsafe, child_process API.
  4. pkg.scripts field actions will have no chance to attack the computer.
  5. pure module can only require pure module.

soft additional design objective:

  1. maybe we need "pureAsync", different with "pure(Sync)", to give a way auto force run in "Worker"-like thing, to protect "while ( true ) ;" (and bind to cpu different with main app context). not very important, because it's almost no benefit for author, and easy to be found by the most careless user (no different with a lib has no user just because it's too slow).
  2. maybe we need consider "import" solution together, like "import mod from 'x' assert { pure: 'sync' }".

LongTengDao avatar Dec 29 '18 08:12 LongTengDao

This is a good idea, but we need a SES-like protection to prevent the globals tempering. I need to check what is the status here. @LongTengDao would you start working on that?

vdeturckheim avatar Jan 09 '19 06:01 vdeturckheim

@vdeturckheim My pleasure. But I'm an user land coder, I'm good at JavaScript, but know nothing about C++, and the JavaScript part source code of Node.js is also so huge, I have almost no idea about the engine level things. If I can help in any case (API discussion?), I'll be glad to do.

LongTengDao avatar Jan 11 '19 02:01 LongTengDao

@hax @i5ting @island205 @JacksonTian @justjavac Would you provide some suggestion? 🛩

LongTengDao avatar Jan 12 '19 07:01 LongTengDao

@LongTengDao It's a very interesting idea. I plan to first try to figure out whether we can implement it in user land (though I don't know how to achieve point 4.)

hax avatar Jan 16 '19 08:01 hax

though I don't know how to achieve point 4.

@hax Thank you, predecessor!

We usually inevitably use scripts field for self building after install it. We can limit it can only use fs to i/o in the module directory, in addition to this, the scripts must run in "pure" mode. If it's a bash... Maybe we can only via an auto system user which only has local i/o permission in the module directory.

Point out, if I misunderstand of scripts field.

LongTengDao avatar Jan 16 '19 17:01 LongTengDao

This is a good idea, but we need a SES-like protection to prevent the globals tempering. I need to check what is the status here.

@vdeturckheim I can't find information about "SES" by Google, what does that mean? And I found "Chrome Content Scripts" in Electron docs, do you mean that? Thanks!

contextIsolation Boolean (optional) - Whether to run Electron APIs and the specified preload script in a separate JavaScript context. Defaults to false. The context that the preload script runs in will still have full access to the document and window globals but it will use its own set of JavaScript builtins (Array, Object, JSON, etc.) and will be isolated from any changes made to the global environment by the loaded page. The Electron API will only be available in the preload script and not the loaded page. This option should be used when loading potentially untrusted remote content to ensure the loaded content cannot tamper with the preload script and any Electron APIs being used. This option uses the same technique used by Chrome Content Scripts. You can access this context in the dev tools by selecting the 'Electron Isolated Context' entry in the combo box at the top of the Console tab.

LongTengDao avatar Feb 19 '19 11:02 LongTengDao

@LongTengDao sure, here is a wiki page https://github.com/google/caja/wiki/SES

you can also check the current state of the realms proposal on TC39 https://github.com/tc39/proposal-realms

vdeturckheim avatar Feb 19 '19 12:02 vdeturckheim

@vdeturckheim Oh, it's cool! Realms is good for plugins of software like vscode. Maybe it's a solution behind require, but I can't see how to use it directly for most module requiring without support of node, am I right? Should we wait realms for meeting this need, or still do it with SES at now?

LongTengDao avatar Feb 19 '19 17:02 LongTengDao

I found a related repo today: https://github.com/secure-require/secure-require

Hope it works, if it's possible in userland.

LongTengDao avatar Aug 14 '19 11:08 LongTengDao

@vdeturckheim @hax https://github.com/tc39/proposal-shadowrealm/issues/347 realms proposal seems won't do what here said, it's still a sticky problem (ShadowRealm API won't have extra limit to import behaviour inside)

LongTengDao avatar Jan 20 '22 06:01 LongTengDao

@LongTengDao Yeah, shadow realms itself can not satisfy what u want in this issue, though it could be the basis of SES in long term. Currently it seems very hard to achieve point 2 without performance burden.

hax avatar Jan 22 '22 16:01 hax

@hax if performance burden is the main problem, then maybe here is a compromise way:

imagine an app, has two directly third-party dependency we call them "x" and "y" here, which expeced to be pure, and "x" has 100 dependencies behind it, "y" too.

then we can only create two isolated context, one for "x" and all it's dependencies, one for "y" and all it's dependencies, that means there is no need to create 202 isolated contexts.

because "pure module" idea mainly protect the main business part, and not intended to promise lib work completely right, including a lib is broken by it's furthur dependencies.

above based on that, "performance burden" you said refer mainly to "create context", not for "run in context". if you mainly mean "run in context", I don't know much, I can only imagine that by "Proxy", I think a cost like "Proxy" is not "too heavy"?

LongTengDao avatar Jan 23 '22 02:01 LongTengDao

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

github-actions[bot] avatar Jul 20 '22 00:07 github-actions[bot]

still waiting to discuss

LongTengDao avatar Jul 21 '22 05:07 LongTengDao

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

github-actions[bot] avatar Nov 02 '22 00:11 github-actions[bot]

It seems a feature request, feel free to join the Security WG to share your point of view. Honestly, I see it overlapping with #791.

RafaelGSS avatar Nov 10 '22 15:11 RafaelGSS

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

github-actions[bot] avatar Feb 09 '23 00:02 github-actions[bot]