Re: [webkit-dev] ExecState::thisObject()

2009-07-14 Thread Geoffrey Garen
That's correct. Other browser's get this case right. Here are a couple test cases you might find interesting: http://webblaze.org/abarth/tests/protoconfused/test1.html http://webblaze.org/abarth/tests/protoconfused/test2.html I tried these tests, with mixed results: IE8: Exception thrown

Re: [webkit-dev] ExecState::thisObject()

2009-07-14 Thread Adam Barth
On Tue, Jul 14, 2009 at 12:31 PM, Geoffrey Garengga...@apple.com wrote: Also, once we've established the model, we'll need to propose it to some standards body -- probably HTML5. I believe the correct spec to describe this behavior is WebIDL, which controls how the abstract DOM interfaces are

Re: [webkit-dev] ExecState::thisObject()

2009-07-14 Thread Geoffrey Garen
Also, once we've established the model, we'll need to propose it to some standards body -- probably HTML5. I believe the correct spec to describe this behavior is WebIDL, which controls how the abstract DOM interfaces are realized in ECMAScript. Sounds good. Geoff

Re: [webkit-dev] ExecState::thisObject()

2009-07-14 Thread Eric Seidel
I've made some initial stabs: https://bugs.webkit.org/show_bug.cgi?id=27276 The bindings need a bunch more cleanup before we can do much more than that. I've started filing bugs about the cleanup. On Tue, Jul 14, 2009 at 12:41 PM, Geoffrey Garengga...@apple.com wrote: Also, once we've

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
1. Is it correct for the ExecState to carry the thisValue? As Adam realized later (I think), ExecState carries the value for this inside the calling function. It does not carry the object whose property is being accessed. Geoff ___ webkit-dev

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Eric Seidel
Re-sending from correct address. On Mon, Jul 13, 2009 at 1:23 PM, Eric Seidelesei...@google.com wrote: I'm looking at this more today. I'm first fixing JSCell::new subclasses to make sure they're always allocating in the correct heap.  If we're to map from objects to the associated

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
Our current thinking is to add a parameter to toJS() to receive the JSGlobalObject in which to create the wrapper. Seems logical. Once we do that, the question is how to find the proper JSGlobalObject at each call site. In most cases, you have another JSObject sitting around that is from

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
Is it definitely right for document.body to make a wrapper using prototypes from the document's host window, rather than from the accessing function's window? What do other browsers do? That's correct. Other browser's get this case right. Is there a particular security or other benefit

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
I'm first fixing JSCell::new subclasses to make sure they're always allocating in the correct heap. If we're to map from objects to the associated globalobject via the heap, we need to fix allocation first. I'm not sure what you guys have been meaning by the phrase correct heap. Barring

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Eric Seidel
On Mon, Jul 13, 2009 at 1:36 PM, Geoffrey Garengga...@apple.com wrote: I'm not sure what you guys have been meaning by the phrase correct heap. Barring workers, all WebCore objects are allocated from the same heap. We had wrongly assumed that each window got its own. OK. This invalidates

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Sam Weinig
On Mon, Jul 13, 2009 at 1:57 PM, Eric Seidel e...@webkit.org wrote: On Mon, Jul 13, 2009 at 1:36 PM, Geoffrey Garengga...@apple.com wrote: I'm not sure what you guys have been meaning by the phrase correct heap. Barring workers, all WebCore objects are allocated from the same heap. We had

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Eric Seidel
On Mon, Jul 13, 2009 at 2:18 PM, Sam Weinigsam.wei...@gmail.com wrote: I discussed this a bit with Darin and Geoff, and we came to the conclusion that the correct fix is to have each JS DOMObject store a JSGlobalObject pointer and augment the toJS methods to pass a global object instead of an

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
1. Pass a current global object through to all toJS calls (lots of callsites changed) 2. Store a current global object off of the ExecState (set by the JS engine before leaving into custom native code for property lookup or function execution). I discussed this a bit with Darin and Geoff, and

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Maciej Stachowiak
On Jul 13, 2009, at 2:18 PM, Sam Weinig wrote: I discussed this a bit with Darin and Geoff, and we came to the conclusion that the correct fix is to have each JS DOMObject store a JSGlobalObject pointer and augment the toJS methods to pass a global object instead of an ExecState (close

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
I discussed this a bit with Darin and Geoff, and we came to the conclusion that the correct fix is to have each JS DOMObject store a JSGlobalObject pointer and augment the toJS methods to pass a global object instead of an ExecState (close to you #1). You probably mean in addition to

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
I discussed this a bit with Darin and Geoff, and we came to the conclusion that the correct fix is to have each JS DOMObject store a JSGlobalObject pointer and augment the toJS methods to pass a global object instead of an ExecState (close to you #1). There are classes in JavaScriptCore

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Adam Barth
On Mon, Jul 13, 2009 at 1:33 PM, Geoffrey Garengga...@apple.com wrote: Is it definitely right for document.body to make a wrapper using prototypes from the document's host window, rather than from the accessing function's window? What do other browsers do? That's correct.  Other browser's

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Adam Barth
On Mon, Jul 13, 2009 at 2:26 PM, Maciej Stachowiakm...@apple.com wrote: For the few cases where cross-origin access is allowed, we would *not* want to expose the home window's prototype chain. So for Window.postMessage for instance, cross-origin access need to give you a distinct wrapper.

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
Is there a particular security or other benefit here, or do we just want to make this change to match other browsers? Our current behavior is buggy, unpredictable, and out of spec. This has led to security bugs in the past and will lead to security bugs in the future. I don't disagree with

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
I discussed this a bit with Darin and Geoff, and we came to the conclusion that the correct fix is to have each JS DOMObject store a JSGlobalObject pointer and augment the toJS methods to pass a global object instead of an ExecState (close to you #1). I would not advocate storing more

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Adam Barth
On Mon, Jul 13, 2009 at 3:29 PM, Geoffrey Garengga...@apple.com wrote: Our current behavior is buggy, unpredictable, and out of spec.  This has led to security bugs in the past and will lead to security bugs in the future. I don't disagree with you, but I'm not immediately convinced that a

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
Which spec did you have in mind? I'd like to read it. Essentially, the ECMAScript spec requires this. In spec-land, these objects are all created at the beginning of time. The fact that we create them lazily is what leads to this bug. Depending on who touches them first, they end up with

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
Yep... My guess is that (a) is probably better, but you two are more expert on why we use lazy construction. Heh -- lazy construction in JSC is even older than I am! :) The goal is to reduce memory use and startup cost in cases where certain functions aren't used. I'm not sure how much of a

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Maciej Stachowiak
On Jul 13, 2009, at 3:36 PM, Geoffrey Garen wrote: I discussed this a bit with Darin and Geoff, and we came to the conclusion that the correct fix is to have each JS DOMObject store a JSGlobalObject pointer and augment the toJS methods to pass a global object instead of an ExecState

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Geoffrey Garen
Consider this case, which does not involve a DOM object: frames[0].Array.prototype.push.__proto__ == Array.prototype.push.__proto__ Built-in classes work somewhat differently. I believe they use the calling function's global object (lexical global object) rather than having some notion

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Adam Barth
On Mon, Jul 13, 2009 at 4:01 PM, Geoffrey Garengga...@apple.com wrote: That's correct.  Other browser's get this case right.  Here are a couple test cases you might find interesting: http://webblaze.org/abarth/tests/protoconfused/test1.html

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Maciej Stachowiak
On Jul 13, 2009, at 3:40 PM, Adam Barth wrote: On Mon, Jul 13, 2009 at 3:29 PM, Geoffrey Garengga...@apple.com wrote: Our current behavior is buggy, unpredictable, and out of spec. This has led to security bugs in the past and will lead to security bugs in the future. I don't disagree

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Adam Barth
On Mon, Jul 13, 2009 at 4:08 PM, Maciej Stachowiakm...@apple.com wrote: Built-in classes work somewhat differently. I believe they use the calling function's global object (lexical global object) rather than having some notion of home object. You might be thinking of boxing primitive values,

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Maciej Stachowiak
On Jul 13, 2009, at 4:12 PM, Adam Barth wrote: On Mon, Jul 13, 2009 at 4:08 PM, Maciej Stachowiakm...@apple.com wrote: Built-in classes work somewhat differently. I believe they use the calling function's global object (lexical global object) rather than having some notion of home

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Maciej Stachowiak
On Jul 13, 2009, at 4:11 PM, Adam Barth wrote: On Mon, Jul 13, 2009 at 4:01 PM, Geoffrey Garengga...@apple.com wrote: That's correct. Other browser's get this case right. Here are a couple test cases you might find interesting: http://webblaze.org/abarth/tests/protoconfused/test1.html

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Adam Barth
On Mon, Jul 13, 2009 at 4:20 PM, Maciej Stachowiakm...@apple.com wrote: My own interest would be in weighing the tradeoffs. In the Pro column: 1) Are there aspects of this issue that create security holes? CVE-2009-1702 is an example of such as security hole. I'm sure that I can find more if

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Maciej Stachowiak
On Jul 13, 2009, at 4:34 PM, Adam Barth wrote: CVE-2009-1702 is an example of such as security hole. I'm sure that I can find more if I look for them. I think objects attached to the global object and accessible cross- origin are a special case here. (The advisory cited is about Location

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Adam Barth
On Mon, Jul 13, 2009 at 4:59 PM, Maciej Stachowiakm...@apple.com wrote: If security is one motivation for this work, then I'd like us to understand the pattern we want to use for cross-origin-accessible objects. Should they use the home global object prototype but protect it from mutation or

Re: [webkit-dev] ExecState::thisObject()

2009-07-13 Thread Maciej Stachowiak
On Jul 13, 2009, at 5:34 PM, Adam Barth wrote: On Mon, Jul 13, 2009 at 4:59 PM, Maciej Stachowiakm...@apple.com wrote: If security is one motivation for this work, then I'd like us to understand the pattern we want to use for cross-origin-accessible objects. Should they use the home

Re: [webkit-dev] ExecState::thisObject()

2009-07-11 Thread Adam Barth
On Fri, Jul 10, 2009 at 6:29 PM, Maciej Stachowiakm...@apple.com wrote: On Jul 10, 2009, at 6:10 PM, Adam Barth wrote: That's correct.  Other browser's get this case right.  Here are a couple test cases you might find interesting: http://webblaze.org/abarth/tests/protoconfused/test1.html

Re: [webkit-dev] ExecState::thisObject()

2009-07-10 Thread Adam Barth
Re-sent with correct address. On Fri, Jul 10, 2009 at 5:55 PM, Adam Barthaba...@eecs.berkeley.edu wrote: Eric and I spent some more time this afternoon looking at this.  We don't think the ExecState::thisValue() approach is going to work.  We implemented hacky version to experiment with, but

Re: [webkit-dev] ExecState::thisObject()

2009-07-10 Thread Maciej Stachowiak
On Jul 10, 2009, at 5:55 PM, Adam Barth wrote: Eric and I spent some more time this afternoon looking at this. We don't think the ExecState::thisValue() approach is going to work. We implemented hacky version to experiment with, but the problem is with cases like this: document.body In

Re: [webkit-dev] ExecState::thisObject()

2009-07-10 Thread Adam Barth
Sent again from the right address. Gmail hates me today. On Fri, Jul 10, 2009 at 6:10 PM, Adam Barthaba...@eecs.berkeley.edu wrote: On Fri, Jul 10, 2009 at 6:04 PM, Maciej Stachowiakm...@apple.com wrote: On Jul 10, 2009, at 5:55 PM, Adam Barth wrote: Eric and I spent some more time this

Re: [webkit-dev] ExecState::thisObject()

2009-07-10 Thread Maciej Stachowiak
(Trimming Cc's since the relevant people are on webkit-dev anyway.) On Jul 10, 2009, at 6:10 PM, Adam Barth wrote: On Fri, Jul 10, 2009 at 6:04 PM, Maciej Stachowiakm...@apple.com wrote: Is it definitely right for document.body to make a wrapper using prototypes from the document's host