-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
How should browsers mitigate Proxy-related security issues? #272
Comments
I find that the Firefox approach (the first one listed) is easier to reason about. Also, the failure happens at writing time rather than reading time. (BTW, nonextensibility and prototype-immutability happen to be bound today, but it wasn't always the case: before |
I think we should try to limit the impact on ECMAScript as much as possible. It would be sad if this issue ended up having wide impacts on ECMAScript when there is no problem for ECMAScript environments that are not the browser. I personally like the FF approach best too. Assuming it's web compatible, the easiest way to spec is probably to teach ES the difference between non-extensible and proto-frozen. This feature, done right, would require its own proposal as it would likely want to include reflective capabilities. However, in the short term, we can add non-observable machinery for it (I guess a new MOP flag and corresponding updates to 9.1.2 [[SetPrototypeOf]] for ordinary objects). |
@annevk I am not sure but it seems like you are talking about a completely different thing than what this issue is discussing. For example there are no cross-origin objects involved here, only cross-origin script loads. |
Firefox has shipped fix 1 in nightly builds for several months, Aurora builds for a couple months, and beta builds for a couple weeks. Come January 26, it'll be shipping in release builds. Early to mid-February I'm going to write up a proposal and spec language for what I implemented (or very nearly -- there are one or two expedient fillips I'd change before standardizing). To briefly sketch the idea. I added a new One tweak we don't implement yet, is making (For completeness: I would be remiss in noting that all of this is really dancing around the issue, that cross-origin script loads ignore MIME type to run the resulting "script" no matter what. It's beyond TC39's ken, but really we have to prohibit mistyped cross-origin script loads to fully fix this. Doable now? Probably not. But neither was mixed-content blocking. And this is exactly akin to that -- moreso, even, because running someone else's code is much worse than displaying an untrustworthy image. We'll just have to deprecate, announce, ship warning messages, ship harsher messages, etc. until we can fully prohibit.) |
Extending the MOP is a high visibility/high impact change and something that should be casually done. Plus, I don't see why a new MOP operations is necessary (or desirable) in order to implement fix 1. If the HTML provided global object and all objects on its initial prototype chain (including %ObjectPrototype%) are considered to be exotic objects, then their [[SetPrototypeOf]] behavior can be to always return false on attempts to modify their current [[Prototype]] value. It would still be possible for such objects to have a back-channel way to initially set or even to dynamically modify their [[Prototype]] value. But that back channel is part of the implementation (or HTML spec.) rather than part of the ES MOP. The only thing I see in the current ES spec. that stands in the way of that is the requirement in 19.1.3 that: "The Object prototype object is an ordinary object." That requirement could be easily deleted or replaced with a statement like: "It is implementation defined whether the Object prototype object is an ordinary object or an exotic object." Some other points: The current ES spec. currently does not require that the global object's prototype chain include %ObjectPrototype%. So, strictly speaking any code that assumes that it does is not guaranteed to be interoperable across standards-conforming ES implementations. I believe that we left it that way when working on the ES5 spec. because IE's global object did not inherit from %ObjectPrototype%. Perhaps it is now time to add to the ES spec. the requirement that the global object inherits form %ObjectPrototype%. Is it ever desirable to allow user code to dynamically change the value of %ObjectPrototype%'s [Prototype]]? I dubious that it is. Rather than making [[Prototype]] mutability of %ObjectPrototype% implementation defined we could define a new kind of standard exotic object with an immutable [[Prototype]] and make %ObjectPrototype% that kind of object. |
I strongly support this. I agree that it's probably a good thing to disallow in general. And furthermore I think that as a general guideline, basic things like Object.prototype should behave the same across all environments as much as possible. Making browsers the only ones with this behavior would be a bit sad IMO. |
We can make %ObjectPrototype% immutable across all environments while at the same time adding a new MOP operation. @allenwb 's suggestion sounds pretty readily implementable, and it'd fix the security issue, but it also sounds like a one-off exception that makes things more complex with funny edge cases, rather than more regular and predictable. I thought we wanted to move away from explaining builtins as special and towards general, regular behavior when possible. |
@littledan One of the primary motivations for the MOP (and its reifaction via Proxy) was to provide a way to explain special case behavior of built-ins and some host objects. This is perfect example of where the current MOP can do exactly that. An object with an immutable prototype reference is simply one where its [[GetPrototypeOf]] internal method always returns the same value and where its [[SetPrototypeOf]] always returns false (except when the value passed is the current value). We want the MOP to be as narrow and simple as possible. The MOP should only consist of the interfaces that are necessary to support the actual semantics of language features and built-in functions. So far, in this thread, I haven't seen anything that requires (or motivates) the introduction of a new language feature or built-in. [[GetPrototypeOf]] and [[SetPrototypeOf]] are already sufficient to support the use case discussed here If you really want to add a [[FreezePrototypeOf]] MOP operation you should start by justifying the addition of an Object.freezePrototypeOf function. In particular, you need to present compelling use cases for why code (probably library code) may need to freeze the prototype reference of arbitrary objects. There may well be a use for such an operation but this the situation addressed in this thread doesn't require it. |
@jswalden Have you written any spec text for this idea, or for the simpler idea of just making Object.prototype exotic? |
This patch builds a mechanism to fix the Proxy security issue documented in bug tc39#272 by locking down the prototype chain of the global object, as Firefox has experimented with. Although the global object is provided by the embedding environment, many embedding environments will include Object in the prototype chain; preventing modification of Object.prototype addresses the issue by making it impossible to insert a Proxy in that part of the prototype chain of the global object. Embedding environments that want to prohibit a Proxy from being in the proto chain of their global object can make their global object and associated proto chain Immutable Prototype exotic objects.
This patch builds a mechanism to fix the Proxy security issue documented in bug tc39#272 by locking down the prototype chain of the global object, as Firefox has experimented with. Although the global object is provided by the embedding environment, many embedding environments will include Object in the prototype chain; preventing modification of Object.prototype addresses the issue by making it impossible to insert a Proxy in that part of the prototype chain of the global object. Embedding environments that want to prohibit a Proxy from being in the proto chain of their global object can make their global object and associated proto chain Immutable Prototype exotic objects.
While extending the MOP is indeed momentous, I think it's a very reasonable choice here. Conflating extensibility with prototype-mutability was always a dodge, done that way simply because people wanted to be able to depend on prototype identity being constant, and that was the only method at hand whose semantics could be plausibly munged to do it. Splitting the two operations (or rather, having function Point(x, y)
{
this.x = x;
this.y = y;
// natural, consistent w/other MOP behavior twiddling on objects
Object.setImmutablePrototype(this);
}
function DumbPoint(x, y)
{
this.x = x;
this.y = y;
// ...really? Extra object, extra overhead to use (in the very real case of
// JITs being unable to boil away overhead), less scrutable in debuggers, etc.
return new Proxy(this, { setPrototypeOf() { return false; } });
} @littledan I haven't written up anything yet, either proposal-language or spec-language, no. |
@jswalden I'm not taking a position on whether or not there is sufficient utility of aObject.setImmutablePrototype function (regardless of its actual name). I'm just same that it isn't needed to address this issue. I suggest that we here we stick with whether we want %ObjectPrototype% to have an immutable [[Prototype]] and assuming so, specifying it in the least intrusive why current at our disposal. You or anyone else should feel free to separately propose a more general mechanism for reflectively controlling prototype mutability. |
@jswalden I'm definitely sympathetic here. Setting an immutable prototype could be a good tool for library authors, and is definitely easier for implementations to take advantage of than Proxy-based prototype immutability. I'd also really like to figure out what our strategy should be for Proxy security to make sure we're not digging ourselves into a hole by releasing something that will have to be locked down somehow later. Do you think you'd be able to come to the TC39 meeting next week to discuss the issue? I put it on the agenda. |
It seems like @littledan's #308 could end up being a subset of a more general set-immutable-prototype hook? That is, if we were to accept #308 for now, and work through a proposal for set-immutable-prototype later, we'd be able to rephrase the definition of Object.prototype from "immutable prototype exotic object" to an ordinary object that has been SetImmutablePrototype-ified, with no change in observable semantics. |
@domenic exactly. If we want a quick fix, the exotic object is the way to go for now. |
@littledan I could come, certainly. Ideally not every day, if there are only a couple proposals I could meaningfully comment on, tho -- any idea what's happening when, or should I just say any day works on the registration thing and wing it? |
@jswalden "Time constraint: Wednesday only" was written on the agenda in tc39/agendas@e57568f . Does that work for you (maybe it was written with you in mind, just wanted to confirm)? |
@littledan Yep, that was me in mind. (Technically I'm available any day, but with few proposals I'd meaningfully comment on, it seemed best for productivity to minimize attendance. Wednesday was the day arbitrarily selected for that purpose. In the unlikely event a scheduling issue arises, Tuesday/Thursday would also do.) |
There's a security issue on the web with ES2015 Proxies where a cross-origin request can be made to load some ECMAScript code, and this request can leak some information across origins due to the existence of Proxies. Details can be found here: https://code.google.com/p/chromium/issues/detail?id=399951 . The short story is that the Proxy can be put in the prototype chain of the global object, and then it's possible to see an identifier-like thing at the beginning of the target page. Making cross-origin requests with non-cooperating sites (by inserting script tags) can leak information which ambient authority signals (like the IP address) grant access to. Telemetry data shows, unfortunately, that it's not web-compatible to lock down mimetype checking for this purpose as I was hoping.
There are a few possible fixes. All require changes to HTML, but they also require deep hooks into ECMAScript.
I like the first two options the best, but I'm really open to any of them, or any additional ideas that people here might have. I'd really like guidance from this committee to move towards one of these, given the tight coupling all of these fixes have with ECMAScript layering.
The text was updated successfully, but these errors were encountered: