Proxy target/handler access leak in Node

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

Proxy target/handler access leak in Node

Darien Valentine
A few weeks ago I’d commented on an open Node github issue regarding Proxies and inspection. While the bulk of the comment concerns an opinion that proxies should not be treated as special case, I included an example of a mechanism by which the current implementation allows outside code to access the target and handler objects of a proxy that it does not own.

On reflection I realized this specific issue might be worth drawing more attention to.

```js
const util = require('util');

const victim = new Proxy({}, {
  SECRET: 'Nothing outside can access this'
});

let secret;

const invariantViolator = {
  [util.inspect.custom](depth, options) {
    const { stylize } = options;

    options.showProxy = true;

    options.stylize = (value, color) => {
      secret = value;
      options.stylize = stylize;
      return stylize(value, color);
    };

    return victim;
  }
};

util.inspect(invariantViolator);

console.log(secret); // 'Nothing outside can access this'
```

The implication is that even if running Node with no C++ addons, it is presently possible for proxies to be violated using just the standard lib, which may be significant from a security perspective. I’m not sure if that’s the case in practice, but just in case, I figured I should try to get eyes on it.

Note that even if this particular hole is patched, the "analog hole" (so to speak) of just analyzing the string output remains.

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Mike Samuel
Nicely done!

One more reason to prefer WeakMaps to properties when relating objects and secrets.



On Sun, Sep 16, 2018 at 2:59 PM Darien Valentine <[hidden email]> wrote:
A few weeks ago I’d commented on an open Node github issue regarding Proxies and inspection. While the bulk of the comment concerns an opinion that proxies should not be treated as special case, I included an example of a mechanism by which the current implementation allows outside code to access the target and handler objects of a proxy that it does not own.

On reflection I realized this specific issue might be worth drawing more attention to.

```js
const util = require('util');

const victim = new Proxy({}, {
  SECRET: 'Nothing outside can access this'
});

let secret;

const invariantViolator = {
  [util.inspect.custom](depth, options) {
    const { stylize } = options;

    options.showProxy = true;

    options.stylize = (value, color) => {
      secret = value;
      options.stylize = stylize;
      return stylize(value, color);
    };

    return victim;
  }
};

util.inspect(invariantViolator);

console.log(secret); // 'Nothing outside can access this'
```

The implication is that even if running Node with no C++ addons, it is presently possible for proxies to be violated using just the standard lib, which may be significant from a security perspective. I’m not sure if that’s the case in practice, but just in case, I figured I should try to get eyes on it.

Note that even if this particular hole is patched, the "analog hole" (so to speak) of just analyzing the string output remains.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Mark Miller-2
This is indeed quite scary. I have never used of explored the Node `util` API. What is going on here? Can you explain?

The Realms shim and SES (which build on the Realms shim) create an environment in which only those globals defined by EcmaScript, not by the host, are present by default. 
Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?



On Sun, Sep 16, 2018 at 12:10 PM Mike Samuel <[hidden email]> wrote:
Nicely done!

One more reason to prefer WeakMaps to properties when relating objects and secrets.



On Sun, Sep 16, 2018 at 2:59 PM Darien Valentine <[hidden email]> wrote:
A few weeks ago I’d commented on an open Node github issue regarding Proxies and inspection. While the bulk of the comment concerns an opinion that proxies should not be treated as special case, I included an example of a mechanism by which the current implementation allows outside code to access the target and handler objects of a proxy that it does not own.

On reflection I realized this specific issue might be worth drawing more attention to.

```js
const util = require('util');

const victim = new Proxy({}, {
  SECRET: 'Nothing outside can access this'
});

let secret;

const invariantViolator = {
  [util.inspect.custom](depth, options) {
    const { stylize } = options;

    options.showProxy = true;

    options.stylize = (value, color) => {
      secret = value;
      options.stylize = stylize;
      return stylize(value, color);
    };

    return victim;
  }
};

util.inspect(invariantViolator);

console.log(secret); // 'Nothing outside can access this'
```

The implication is that even if running Node with no C++ addons, it is presently possible for proxies to be violated using just the standard lib, which may be significant from a security perspective. I’m not sure if that’s the case in practice, but just in case, I figured I should try to get eyes on it.

Note that even if this particular hole is patched, the "analog hole" (so to speak) of just analyzing the string output remains.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss


--
  Cheers,
  --MarkM

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Darien Valentine
> What is going on here? Can you explain?

A C++/V8 API is used to obtain references to the target and handler from only the proxy object, even though those objects aren’t supposed to be available to this ES scope:


The pair of objects is passed to another function, which in turn may pass values to the method "ctx.stylize", but that property may have been previously overwritten from user code:


AFAICT, fixing the leak (beyond util.js) would be possible by making `stylize` unconfigurable/unwritable. However there may be other avenues here — I’m not sure.

> Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?

The exploit isn’t sensitive to realm-of-origin, since util is using v8 internals to get the references, not e.g. a patched version of Proxy. But it still depends on the proxy object in question being exposed to a context where util.inspect exists.


> This exploit can be fixed pretty easily. But I think it’s symptomatic. My understanding was that V8’s C++ APIs are intended for making external functionality available to ECMAScript using ECMAScript values and ECMAScript semantics. Here though, it is being used to bypass ECMAScript semantics. The behavior seen in the above script isn’t possible in ECMAScript — but neither is distinguishing between objects which have been implemented as ordinary objects and objects implemented as proxy exotic objects.


On Sun, Sep 16, 2018 at 8:06 PM Mark Miller <[hidden email]> wrote:
This is indeed quite scary. I have never used of explored the Node `util` API. What is going on here? Can you explain?

The Realms shim and SES (which build on the Realms shim) create an environment in which only those globals defined by EcmaScript, not by the host, are present by default. 
Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?



On Sun, Sep 16, 2018 at 12:10 PM Mike Samuel <[hidden email]> wrote:
Nicely done!

One more reason to prefer WeakMaps to properties when relating objects and secrets.



On Sun, Sep 16, 2018 at 2:59 PM Darien Valentine <[hidden email]> wrote:
A few weeks ago I’d commented on an open Node github issue regarding Proxies and inspection. While the bulk of the comment concerns an opinion that proxies should not be treated as special case, I included an example of a mechanism by which the current implementation allows outside code to access the target and handler objects of a proxy that it does not own.

On reflection I realized this specific issue might be worth drawing more attention to.

```js
const util = require('util');

const victim = new Proxy({}, {
  SECRET: 'Nothing outside can access this'
});

let secret;

const invariantViolator = {
  [util.inspect.custom](depth, options) {
    const { stylize } = options;

    options.showProxy = true;

    options.stylize = (value, color) => {
      secret = value;
      options.stylize = stylize;
      return stylize(value, color);
    };

    return victim;
  }
};

util.inspect(invariantViolator);

console.log(secret); // 'Nothing outside can access this'
```

The implication is that even if running Node with no C++ addons, it is presently possible for proxies to be violated using just the standard lib, which may be significant from a security perspective. I’m not sure if that’s the case in practice, but just in case, I figured I should try to get eyes on it.

Note that even if this particular hole is patched, the "analog hole" (so to speak) of just analyzing the string output remains.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss


--
  Cheers,
  --MarkM

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Isiah Meadows-2
In the browser, I could see why not to allow it by default: it's a
potential security hole, and GC concerns do exist (what happens if the
symbol's data is no longer accessible without reflection?).

In embeddings, I'm not sure if there's any real problem, considering
Node's `vm` API allows proper sandboxing to prevent scripts from
getting ahold of a `util` object in the first place, and by extension,
the private data assigned to it.

It might be worth accepting a level of non-determinism within the spec
stating that if a symbol is deemed not accessible by the
implementation, they aren't required to return it. Objects would have
to hold those private symbols weakly, but the non-determinism would
need to clarified with the weak ref proposal. (This is primarily a
concern with private symbols, and an implementation might choose not
to expose that in the first place, even to embedders.)

-----

Isiah Meadows
[hidden email]
www.isiahmeadows.com

On Sun, Sep 16, 2018 at 9:19 PM Darien Valentine <[hidden email]> wrote:

>
> > What is going on here? Can you explain?
>
> A C++/V8 API is used to obtain references to the target and handler from only the proxy object, even though those objects aren’t supposed to be available to this ES scope:
>
> https://github.com/nodejs/node/blob/master/lib/util.js#L642-L647
>
> The pair of objects is passed to another function, which in turn may pass values to the method "ctx.stylize", but that property may have been previously overwritten from user code:
>
> https://github.com/nodejs/node/blob/master/lib/util.js#L566-L580
>
> AFAICT, fixing the leak (beyond util.js) would be possible by making `stylize` unconfigurable/unwritable. However there may be other avenues here — I’m not sure.
>
> > Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?
>
> The exploit isn’t sensitive to realm-of-origin, since util is using v8 internals to get the references, not e.g. a patched version of Proxy. But it still depends on the proxy object in question being exposed to a context where util.inspect exists.
>
> In the [issue comment](https://github.com/nodejs/node/issues/10731#issuecomment-419008987), I wrote this:
>
> > This exploit can be fixed pretty easily. But I think it’s symptomatic. My understanding was that V8’s C++ APIs are intended for making external functionality available to ECMAScript using ECMAScript values and ECMAScript semantics. Here though, it is being used to bypass ECMAScript semantics. The behavior seen in the above script isn’t possible in ECMAScript — but neither is distinguishing between objects which have been implemented as ordinary objects and objects implemented as proxy exotic objects.
>
>
> On Sun, Sep 16, 2018 at 8:06 PM Mark Miller <[hidden email]> wrote:
>>
>> This is indeed quite scary. I have never used of explored the Node `util` API. What is going on here? Can you explain?
>>
>> The Realms shim and SES (which build on the Realms shim) create an environment in which only those globals defined by EcmaScript, not by the host, are present by default.
>> https://github.com/Agoric/SES/tree/master/demo
>> https://rawgit.com/Agoric/SES/master/demo/
>> Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?
>>
>>
>>
>> On Sun, Sep 16, 2018 at 12:10 PM Mike Samuel <[hidden email]> wrote:
>>>
>>> Nicely done!
>>>
>>> One more reason to prefer WeakMaps to properties when relating objects and secrets.
>>>
>>>
>>>
>>> On Sun, Sep 16, 2018 at 2:59 PM Darien Valentine <[hidden email]> wrote:
>>>>
>>>> A few weeks ago I’d commented on an open Node github issue regarding Proxies and inspection. While the bulk of the comment concerns an opinion that proxies should not be treated as special case, I included an example of a mechanism by which the current implementation allows outside code to access the target and handler objects of a proxy that it does not own.
>>>>
>>>> On reflection I realized this specific issue might be worth drawing more attention to.
>>>>
>>>> ```js
>>>> const util = require('util');
>>>>
>>>> const victim = new Proxy({}, {
>>>>   SECRET: 'Nothing outside can access this'
>>>> });
>>>>
>>>> let secret;
>>>>
>>>> const invariantViolator = {
>>>>   [util.inspect.custom](depth, options) {
>>>>     const { stylize } = options;
>>>>
>>>>     options.showProxy = true;
>>>>
>>>>     options.stylize = (value, color) => {
>>>>       secret = value;
>>>>       options.stylize = stylize;
>>>>       return stylize(value, color);
>>>>     };
>>>>
>>>>     return victim;
>>>>   }
>>>> };
>>>>
>>>> util.inspect(invariantViolator);
>>>>
>>>> console.log(secret); // 'Nothing outside can access this'
>>>> ```
>>>>
>>>> The implication is that even if running Node with no C++ addons, it is presently possible for proxies to be violated using just the standard lib, which may be significant from a security perspective. I’m not sure if that’s the case in practice, but just in case, I figured I should try to get eyes on it.
>>>>
>>>> Note that even if this particular hole is patched, the "analog hole" (so to speak) of just analyzing the string output remains.
>>>> _______________________________________________
>>>> es-discuss mailing list
>>>> [hidden email]
>>>> https://mail.mozilla.org/listinfo/es-discuss
>>>
>>> _______________________________________________
>>> es-discuss mailing list
>>> [hidden email]
>>> https://mail.mozilla.org/listinfo/es-discuss
>>
>>
>>
>> --
>>   Cheers,
>>   --MarkM
>
> _______________________________________________
> es-discuss mailing list
> [hidden email]
> https://mail.mozilla.org/listinfo/es-discuss
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

James M Snell
In reply to this post by Darien Valentine
Some background as I am the one who added the showProxy feature into Node.js...

util.inspect() is considered by Node.js to be a diagnostics API. The intent is to allow adequate debugging in a variety of scenarios. This was added to address a user issue where inspection of a Proxy object (that the user didn't know was a proxy object) would cause an infinite loop when invoking a certain getter that just happened to have a console.log statement (in Node.js, console.log uses util.inspect in some cases. There are cases where one does need to know when an object is a Proxy.

That said, the specific issue in this thread is a bit different. When implementing showProxy, I took the additional step of introspecting the component elements of the proxy because the goal of util.inspect is to provide as much insight as possible about the object being inspected. The ability to provide a custome stylize function for inspect is a long standing feature of Node.js long before showProxy was introduced.

One thing I would not classify this as is an attack, exploit, or vulnerability. The Node.js trust model assumes that all code is trusted. I can, however, bring this issue to the Node.js project and see how folks would feel about not exposing the internal objects when a Proxy is detected. That would reduce the diagnostic utility of util.inspect with Proxy objects, which is unfortunate, but it would resolve the issue here. Removing this would likely require a proper deprecation of the existing functionality which is a semver-major change.

The final thing I would say is: Node.js has a responsible disclosure process for potential security vulnerability reports. If one feels that some behavior implemented by node.js presents a vulnerability or attack vector, I would encourage them to please use our HackerOne site (https://hackerone.com/nodejs) to report the issue before discussing in a public forum so that the potential risks can be evaluated before public disclosure.



On Mon, Sep 17, 2018, 02:19 Darien Valentine <[hidden email]> wrote:
> What is going on here? Can you explain?

A C++/V8 API is used to obtain references to the target and handler from only the proxy object, even though those objects aren’t supposed to be available to this ES scope:


The pair of objects is passed to another function, which in turn may pass values to the method "ctx.stylize", but that property may have been previously overwritten from user code:


AFAICT, fixing the leak (beyond util.js) would be possible by making `stylize` unconfigurable/unwritable. However there may be other avenues here — I’m not sure.

> Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?

The exploit isn’t sensitive to realm-of-origin, since util is using v8 internals to get the references, not e.g. a patched version of Proxy. But it still depends on the proxy object in question being exposed to a context where util.inspect exists.


> This exploit can be fixed pretty easily. But I think it’s symptomatic. My understanding was that V8’s C++ APIs are intended for making external functionality available to ECMAScript using ECMAScript values and ECMAScript semantics. Here though, it is being used to bypass ECMAScript semantics. The behavior seen in the above script isn’t possible in ECMAScript — but neither is distinguishing between objects which have been implemented as ordinary objects and objects implemented as proxy exotic objects.


On Sun, Sep 16, 2018 at 8:06 PM Mark Miller <[hidden email]> wrote:
This is indeed quite scary. I have never used of explored the Node `util` API. What is going on here? Can you explain?

The Realms shim and SES (which build on the Realms shim) create an environment in which only those globals defined by EcmaScript, not by the host, are present by default. 
Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?



On Sun, Sep 16, 2018 at 12:10 PM Mike Samuel <[hidden email]> wrote:
Nicely done!

One more reason to prefer WeakMaps to properties when relating objects and secrets.



On Sun, Sep 16, 2018 at 2:59 PM Darien Valentine <[hidden email]> wrote:
A few weeks ago I’d commented on an open Node github issue regarding Proxies and inspection. While the bulk of the comment concerns an opinion that proxies should not be treated as special case, I included an example of a mechanism by which the current implementation allows outside code to access the target and handler objects of a proxy that it does not own.

On reflection I realized this specific issue might be worth drawing more attention to.

```js
const util = require('util');

const victim = new Proxy({}, {
  SECRET: 'Nothing outside can access this'
});

let secret;

const invariantViolator = {
  [util.inspect.custom](depth, options) {
    const { stylize } = options;

    options.showProxy = true;

    options.stylize = (value, color) => {
      secret = value;
      options.stylize = stylize;
      return stylize(value, color);
    };

    return victim;
  }
};

util.inspect(invariantViolator);

console.log(secret); // 'Nothing outside can access this'
```

The implication is that even if running Node with no C++ addons, it is presently possible for proxies to be violated using just the standard lib, which may be significant from a security perspective. I’m not sure if that’s the case in practice, but just in case, I figured I should try to get eyes on it.

Note that even if this particular hole is patched, the "analog hole" (so to speak) of just analyzing the string output remains.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss


--
  Cheers,
  --MarkM
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Darien Valentine
Thanks for the context, James. Yes, this thread mainly concerns the issue of being able to obtain references to values within the handler/target from external code, though I did try to make a case for not having the showProxy option in the original issue thread.

I would also not have thought to call it an “attack” vector. Mark would be able to say better for sure though. It does make an invariant of the language violable though. It’s similar to exposing a function which, given only a function object, may return references to arbitrary values from that function’s scope.

On Mon, Sep 17, 2018 at 5:45 AM James M Snell <[hidden email]> wrote:
Some background as I am the one who added the showProxy feature into Node.js...

util.inspect() is considered by Node.js to be a diagnostics API. The intent is to allow adequate debugging in a variety of scenarios. This was added to address a user issue where inspection of a Proxy object (that the user didn't know was a proxy object) would cause an infinite loop when invoking a certain getter that just happened to have a console.log statement (in Node.js, console.log uses util.inspect in some cases. There are cases where one does need to know when an object is a Proxy.

That said, the specific issue in this thread is a bit different. When implementing showProxy, I took the additional step of introspecting the component elements of the proxy because the goal of util.inspect is to provide as much insight as possible about the object being inspected. The ability to provide a custome stylize function for inspect is a long standing feature of Node.js long before showProxy was introduced.

One thing I would not classify this as is an attack, exploit, or vulnerability. The Node.js trust model assumes that all code is trusted. I can, however, bring this issue to the Node.js project and see how folks would feel about not exposing the internal objects when a Proxy is detected. That would reduce the diagnostic utility of util.inspect with Proxy objects, which is unfortunate, but it would resolve the issue here. Removing this would likely require a proper deprecation of the existing functionality which is a semver-major change.

The final thing I would say is: Node.js has a responsible disclosure process for potential security vulnerability reports. If one feels that some behavior implemented by node.js presents a vulnerability or attack vector, I would encourage them to please use our HackerOne site (https://hackerone.com/nodejs) to report the issue before discussing in a public forum so that the potential risks can be evaluated before public disclosure.



On Mon, Sep 17, 2018, 02:19 Darien Valentine <[hidden email]> wrote:
> What is going on here? Can you explain?

A C++/V8 API is used to obtain references to the target and handler from only the proxy object, even though those objects aren’t supposed to be available to this ES scope:


The pair of objects is passed to another function, which in turn may pass values to the method "ctx.stylize", but that property may have been previously overwritten from user code:


AFAICT, fixing the leak (beyond util.js) would be possible by making `stylize` unconfigurable/unwritable. However there may be other avenues here — I’m not sure.

> Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?

The exploit isn’t sensitive to realm-of-origin, since util is using v8 internals to get the references, not e.g. a patched version of Proxy. But it still depends on the proxy object in question being exposed to a context where util.inspect exists.


> This exploit can be fixed pretty easily. But I think it’s symptomatic. My understanding was that V8’s C++ APIs are intended for making external functionality available to ECMAScript using ECMAScript values and ECMAScript semantics. Here though, it is being used to bypass ECMAScript semantics. The behavior seen in the above script isn’t possible in ECMAScript — but neither is distinguishing between objects which have been implemented as ordinary objects and objects implemented as proxy exotic objects.


On Sun, Sep 16, 2018 at 8:06 PM Mark Miller <[hidden email]> wrote:
This is indeed quite scary. I have never used of explored the Node `util` API. What is going on here? Can you explain?

The Realms shim and SES (which build on the Realms shim) create an environment in which only those globals defined by EcmaScript, not by the host, are present by default. 
Does that mean this attack is impossible from code loaded into a new realm as made by the shim or SES?



On Sun, Sep 16, 2018 at 12:10 PM Mike Samuel <[hidden email]> wrote:
Nicely done!

One more reason to prefer WeakMaps to properties when relating objects and secrets.



On Sun, Sep 16, 2018 at 2:59 PM Darien Valentine <[hidden email]> wrote:
A few weeks ago I’d commented on an open Node github issue regarding Proxies and inspection. While the bulk of the comment concerns an opinion that proxies should not be treated as special case, I included an example of a mechanism by which the current implementation allows outside code to access the target and handler objects of a proxy that it does not own.

On reflection I realized this specific issue might be worth drawing more attention to.

```js
const util = require('util');

const victim = new Proxy({}, {
  SECRET: 'Nothing outside can access this'
});

let secret;

const invariantViolator = {
  [util.inspect.custom](depth, options) {
    const { stylize } = options;

    options.showProxy = true;

    options.stylize = (value, color) => {
      secret = value;
      options.stylize = stylize;
      return stylize(value, color);
    };

    return victim;
  }
};

util.inspect(invariantViolator);

console.log(secret); // 'Nothing outside can access this'
```

The implication is that even if running Node with no C++ addons, it is presently possible for proxies to be violated using just the standard lib, which may be significant from a security perspective. I’m not sure if that’s the case in practice, but just in case, I figured I should try to get eyes on it.

Note that even if this particular hole is patched, the "analog hole" (so to speak) of just analyzing the string output remains.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss


--
  Cheers,
  --MarkM
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Mark Miller-2


On Mon, Sep 17, 2018 at 8:32 AM Darien Valentine <[hidden email]> wrote:
Thanks for the context, James. Yes, this thread mainly concerns the issue of being able to obtain references to values within the handler/target from external code, though I did try to make a case for not having the showProxy option in the original issue thread.

I would also not have thought to call it an “attack” vector. Mark would be able to say better for sure though. It does make an invariant of the language violable though. It’s similar to exposing a function which, given only a function object, may return references to arbitrary values from that function’s scope.

> It’s similar to exposing a function which, given only a function object, may return references to arbitrary values from that function’s scope.

This is an apt comparison. A debugger has access to such info. Likewise, in a secure OS, when one process is able to debug another, the first process can read any data from the address space of the second. There have even been language implementations that were otherwise supposed to be memory safe that had "peek" and "poke" operations for reading and writing arbitrary memory locations from programs in the language. Of course, memory allocators and garbage collectors typically need such access.

Whether these are "attacks" or "vulnerabilities" depends on how such permission for debug-level access or peek/poke access is controlled and provided.

From a bit of web searching, I found the following worrisome:

seemingly in response to

Making is a public symbol in this manner means it is almost impossible to deny. It is still true that "util" is deniable, so this isn't necessarily fatal. I am not yet oriented enough to understand what the consequences are of suppressing util; but I am worried.

--
  Cheers,
  --MarkM

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Mark Miller-2
In reply to this post by James M Snell
> The Node.js trust model assumes that all code is trusted.

First I want to respond to this sentence out of context. I often hear such phrases. I know what people mean by this, but the phrase "trusted" here *always* leads to confusion and muddy thinking. I don't trust the code I wrote yesterday. Instead, what we mean by this is something like:

"The Node.js xxxx model assumes we are fully vulnerable to all code."

This phrasing helps us notice some of the questions made obscure by the earlier phrase. What is fully vulnerable to which code? What is meant in this case is presumably something more like

"...assumes the Node.js process is fully vulnerable to all code it is asked to run."

Under a traditional OS, a process executes as the account (or "user") executing that process, and has all the permissions of that user. So this becomes:

"...assumes the user is fully vulnerable to all code that any Node process executing as that user is asked to run."

(Which of course includes anything built on Electron, which makes the situation even worse in some ways.)

Given the way that this body of code is typically selected, by transitive alleged package dependencies, this is a ridiculously large attack surface. Fortunately, there is increasing appreciation that such pervasive vulnerability is problematic.

See 

Also relevant
 
--
  Cheers,
  --MarkM

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Proxy target/handler access leak in Node

Darien Valentine
> Making is a public symbol in this manner means it is almost impossible to deny. It is still true that "util" is deniable, so this isn't necessarily fatal. I am not yet oriented enough to understand what the consequences are of suppressing util; but I am worried.

I wasn’t under the impression that the util.inspect.custom symbol or the functionality it provides is itself problematic. It just happens to be possible to use it, currently, to get inside the proxy target/handler because its second argument is an object with a property whose value is a function that can be called with proxy target / handler values, yet that property may be overwritten by user code. This is very unlikely to have been an intentional facet of the API, so making `opts.stylize` unwritable/unconfigurable shouldn’t reduce the usefulness of custom inspect implementations, and afaict this would block off the avenue by which references to target/handler can escape util.js.

Is there another reason why the util.inspect.custom symbol contract would be an issue?

On Mon, Sep 17, 2018 at 1:21 PM Mark Miller <[hidden email]> wrote:
> The Node.js trust model assumes that all code is trusted.

First I want to respond to this sentence out of context. I often hear such phrases. I know what people mean by this, but the phrase "trusted" here *always* leads to confusion and muddy thinking. I don't trust the code I wrote yesterday. Instead, what we mean by this is something like:

"The Node.js xxxx model assumes we are fully vulnerable to all code."

This phrasing helps us notice some of the questions made obscure by the earlier phrase. What is fully vulnerable to which code? What is meant in this case is presumably something more like

"...assumes the Node.js process is fully vulnerable to all code it is asked to run."

Under a traditional OS, a process executes as the account (or "user") executing that process, and has all the permissions of that user. So this becomes:

"...assumes the user is fully vulnerable to all code that any Node process executing as that user is asked to run."

(Which of course includes anything built on Electron, which makes the situation even worse in some ways.)

Given the way that this body of code is typically selected, by transitive alleged package dependencies, this is a ridiculously large attack surface. Fortunately, there is increasing appreciation that such pervasive vulnerability is problematic.

See 

Also relevant
 
--
  Cheers,
  --MarkM

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss