w3c / webappsec-secure-contexts

WebAppSec Secure Contexts
https://w3c.github.io/webappsec-secure-contexts/
Other
33 stars 38 forks source link

Consider: isSecureContext should be [Unforgeable] #46

Closed DigiTec closed 6 years ago

DigiTec commented 7 years ago

The Unforgeable attribute is a way to mark a property as living on the instance of an object and being non configurable. For Window, due to Window as Global semantics, all properties live directly on the Global context already and so this will simply mark it as non configurable. The document property is one such example.

Chrome incorrectly marks document as a value (should have a getter instead) but the configurable/writable flags are correct.

Object.getOwnPropertyDescriptor(window, "document")
Object {value: document, writable: false, enumerable: true, configurable: false}
domenic commented 7 years ago

My impression was that [Unforgeable] was kind of a legacy thing? But maybe this is the kind of rare case where it makes sense, if it ever does? @annevk @heycam @bzbarsky

DigiTec commented 7 years ago

I don't think it was legacy because we added the concept for security right before we shipped IE 9. Maybe it was legacy in that before, with COM based window, you couldn't override the document, but the goal with document was to ensure that you could access certain things like URL to ensure that someone wasn't running you in a context in which you did not want to run. isSecureContext might be a check some libraries want to add into their code, same as origin restrictions?

mikewest commented 7 years ago

Chrome uses [Unforgeable] for window.document and the isTrusted attribute of Event objects. I don't think there's a HUGE amount of value to adding it for this attribute, but at the same time, it's a trivial change from a technical perspective.

Since it exists, is supported in multiple browsers, and seems relevant and painless, I'm inclined to add the attribute. Are there architectural questions around its usage?

/cc @slightlyoff @hillbrad

annevk commented 7 years ago

I wish I'd know more about the attack scenario. Generally if you cannot trust properties on an object, you've already lost, even if that one property ends up returning the correct result.

mikewest commented 7 years ago

The scenario mentioned above is a library/widget which might be embedded into various contexts, and offer different functionality based on those conexts' status. I generally agree that the cases in which the unforgeability prevents confusion are a bit contrived, but I also don't see harm in applying this existing infrastructure to the attribute.

domenic commented 7 years ago

I'd like us to get our story straight on [Unforgeable] though. Do we sprinkle it everywhere where it might not cause harm and seems security-related? Or is it a legacy feature from the early days of the web platform when we didn't understand threat models, and it should be renamed [LegacyUnforgeable] and never used again?

mikewest commented 7 years ago

Yeah, for clarity: I agree with @domenic. If we want [Unforgeable], then this seems like a fine place to apply it. If not, then we should be clear about that in the IDL spec. /cc @tobie

annevk commented 6 years ago

https://github.com/heycam/webidl/issues/350 is the IDL issue. Closing this.