valentinium at gmail.com (2018-09-17T15:40:12.682Z)
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 — agreed that from an “all code is trusted” POV, there is no security issue. But Mark would be able to say better for sure, and apologies if this should have been reported to HackerOne after all. What it does is make an invariant of the language violable. It’s similar to exposing a function which, given only a function object, may return references to arbitrary values from that function’s scope.
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 <jasnell at gmail.com> 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 <valentinium at gmail.com> > 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 <erights at gmail.com> 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 <mikesamuel at gmail.com> >>> 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 <valentinium at gmail.com> >>>> 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 >>>>> es-discuss at mozilla.org >>>>> https://mail.mozilla.org/listinfo/es-discuss >>>>> >>>> _______________________________________________ >>>> es-discuss mailing list >>>> es-discuss at mozilla.org >>>> https://mail.mozilla.org/listinfo/es-discuss >>>> >>> >>> >>> -- >>> Cheers, >>> --MarkM >>> >> _______________________________________________ >> es-discuss mailing list >> es-discuss at mozilla.org >> https://mail.mozilla.org/listinfo/es-discuss >> > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20180917/965d65b4/attachment.html>