Objects constructed in another window triggering V8 miscompile(?)
Reported by
ken...@corp.sandstorm.io,
Mar 20 2016
|
|||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.26 Safari/537.36 Steps to reproduce the problem: Unfortunately, this is as Heisenbug. I tried my best to narrow it down but this is the best I could do. 1. Create a window.opener relationship between two tabs, such that one is the window.opener of the other. This happens e.g. when clicking a link with target="_blank". 2. In both tabs, navigate to a Meteor-based site that includes the UnderscoreJS package (version 1.5.2). https://oasis.sandstorm.io and https://docs.meteor.com both seem to work. 3. In the child tab, create an object on the parent window, like: `window.opener.foo = {foo: "123"}` 4. In the parent tab, note that `window.foo` appears to be the correct object. 5. In the parent tab, do: _.each(foo, console.log.bind(console)) What is the expected behavior? The following should be printed to the console: 123 "foo" Object {foo: 123} What went wrong? Instead, nothing is printed. Did this work before? Yes 49 Chrome version: 50.0.2661.26 Channel: beta OS Version: 4.3.0-1-amd64 (Debian) Flash Version: Shockwave Flash 21.0 r0 The bug is not deterministic. In the wild, the symptoms come and go. However, the repro steps above seem to work consistently for me. If I try to copy/paste the minified code into the console and then run it, the bug doesn't occur. Perhaps this is because the "real" copy has been JITed in a special way that is not easy to trigger. In any case, this makes it very hard to narrow down what went wrong. But, I can say the following: Typing "_.each" on the console prints: function (n,t,r){if(null!=n)if(p&&n.forEach===p)n.forEach(t,r);else if(O(n)){for(var u=0,i=n.length;i>u;u++)if(t.call(r,n[u],u,n)===e)return}else for(var a=A.keys(n),u=0,i=a.length;i>u;u++)if(t.call(r… Notice the function "O". By typing {a:_each} to get an object tree view, we can dig into the function scope and extract this function "O" via right-click -> "Store as Global Variable". Chrome will give it the name "temp1". Now let's inspect it. Typing "temp1" says: function (n){return n.length===+n.length&&(k(n)||n.constructor!==Object)} This function appears to be trying to determine whether its parameter is array-like. The first part of the test, "n.length===+n.length", obviously should evaluate false if n.length is undefined, thereby leading the whole function to return false. And indeed, if we type "foo.length===+foo.length" on the console, it tells us "false". However, if we call temp1(foo), it returns true! Clearly, v8 has screwed up somewhere. This bug currently breaks the way Sandstorm interacts with its app market.
,
Mar 22 2016
,
Mar 28 2016
Tested this as per the following test steps: ============================================ 1. Opened https://oasis.sandstorm.io/ 2. Under console entered '_.each' and observed. Actual: ======= VM84:1 Uncaught ReferenceError: _ is not defined at <anonymous>:1:1 at Object.InjectedScript._evaluateOn (<anonymous>:145:167) at Object.InjectedScript._evaluateAndWrap (<anonymous>:137:25) at Object.InjectedScript.evaluate (<anonymous>:118:14) Expected: ========== function (n,t,r){if(null!=n)if(p&&n.forEach===p)n.forEach(t,r);else if(O(n)){for(var u=0,i=n.length;i>u;u++)if(t.call(r,n[u],u,n)===e)return}else for(var a=A.keys(n),u=0,i=a.length;i>u;u++)if(t.call(r… Based on the above test steps, able to reproduce this on Windows-7, Mac OS 10.11.3 & Linux Ubuntu 14.04 on the latest canary(51.0.2692.0) and the latest beta(50.0.2661.49). Regressed in M-50 ================== Last good build: 50.0.2659.0 First bad build: 50.0.2660.0 Changelog: ============ https://chromium.googlesource.com/chromium/src/+log/6863feb517c5d9e4919b7ccfb53fc2a55db76025..12922105a957374c852231405a21de55cad0f028 Suspected change: https://codereview.chromium.org/1668603003 samli@: Could you please take a look at this
,
Mar 28 2016
,
Mar 28 2016
To be clear, the problem I observed manifests as real bugs even when the devtools aren't being used, therefore I don't think this bug is specific to the devtools.
,
Mar 28 2016
A friendly reminder that M50 Stable is launching soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch by Apr-5. All changes MUST be merged into the release branch by 5pm on Apr-8 to make into the desktop Stable final build cut. Thanks!
,
Mar 28 2016
@kenton: could you provide a step-by-step repro? The comment #3 is unrelated to the issue reported: it was using wrong context in the console context selector.
,
Mar 28 2016
@pfeldman My original report contains a step-by-step repro, but I'll try to rewrite it here to be clearer. 1. Open a new tab. 2. Click on any link that has target=_blank in order to create a second tab whose window.opener is the first tab. For example, browse to Google+ and click on any external link. 3. Browse both tabs to https://docs.meteor.com (or https://oasis.sandstorm.io). 4. In the child tab's JS console, type: window.opener.foo = {foo: "123"} 5. In the parent tab's JS console, verify that step 4 created `window.foo`. Typing `foo` should return `Object {foo: "123"}`. 6. In the parent tab's JS console, type and observe the result of: _.each(foo, console.log.bind(console)) 7. In the parent tab's JS console, type and observe the result of: _.each({foo: "123"}, console.log.bind(console)) 6 and 7 should produce exactly the same result, since the object literal `{foo: "123"}` is equivalent to the value stored in `foo` (aka `window.foo`). However, step 6 prints nothing, while step 7 prints `123 foo Object {foo: "123"}`. We can dig a bit further (everything below is in the parent tab): 8. Bring up an object tree inspector by typing: {a: _.each} 9. Browse the object tree to: Object > a > <function scope> > Closure > O 10. Right-click on `O` and choose "Store as Global Variable". (It becomes `temp1`.) 11. Observe the code of temp1, which has been printed on the console: `function (n){return n.length===+n.length&&(k(n)||n.constructor!==Object)}` 12. Type and observe the result of: temp1(foo) 13. Type and observe the result of: temp1({foo: "123"}) Step 12 returns true, but step 13 returns false. If you look at the code (step 11), you can clearly see that temp1 should return false since `foo.length===+foo.length` is obviously false (and indeed, if you type that manually, it evaluates false). Yet, it is returning true.
,
Mar 28 2016
1. Open a new tab. 2. Click on any link that has target=_blank in order to create a second tab whose window.opener is the first tab. For example, browse to Google+ and click on any external link. External link is going to be site (process) isolated and there will be no way to script between them. In order opener to be scriptable, you need to open the second tab using window.open. But even if you do that, navigating them to a new domain one by one breaks that connection. So it seems to work as intended. Please provide more info and I will reopen this issue!
,
Mar 29 2016
@pfeldman You can get the same effect with window.open() and without ever leaving the site! I suggested finding a target="_blank" link to click as a way to set this up because calling window.open() in the JS console triggers the popup blocker. If you then tell the popup blocker to allow the popup, it actually *doesn't* create a window.opener relationship. AFAICT this is an unrelated bug in the popup blocker. If you disable the popup blocker entirely in advance, then you can repro the bug using window.open() from the JS console and never browsing away from the site. Meanwhile, your assertions about window.opener are just not correct. The window.opener relationship *is in fact created* by target="_blank" links and *does in fact persist* when navigating to different origins (although attempts to access window.opener will only succeed if at the time of access they are the same origin). If this were not the case, then my repro steps *would not work as described*: In the steps, I successfully "scripted between" the two tabs. The bug only happened later on, in that the cross-tab object was treated subtly differently from a same-tab object. If scripting between these tabs was not supposed to happen, then there is a much bigger, possibly-security-sensitive bug here! Please reopen this.
,
Mar 29 2016
I can do the following:
- open any static page on domain A
- window.open("/") from console
- manually navigate both to another static page on domain B
- window.opener is still available for scripting
creis@, mkwst@: is that expected?
,
Mar 29 2016
@pfeldman FWIW I believe it's expected under web standards, and all other browsers seem to behave the same. Basically, window.opener is set when the window opens and never changes, but you're only allowed to access it (other than postMessage()) if it's currently in the same origin. But that said, to be clear, it's orthogonal to this bug. This bug happens even if you start on the affected site and never navigate away.
,
Mar 30 2016
I don't get the bug then.
,
Mar 30 2016
I was explained that comment #11 describes intended behavior. Now over to an actual bug... Still need crystal clear repro steps. Screencast might help.
,
Mar 30 2016
Comments 11-12: That's right-- openers are preserved across cross-site navigations (even most cross-process navigations in Chrome). The opener for the window can be disowned by assigning it to null, though. (I haven't looked at the rest of the bug.)
,
Mar 30 2016
@pfeldman Which steps from comment 8 do you feel are not clear? I can try to expand on them or take screenshots.
,
Apr 4 2016
,
Apr 4 2016
M50 Stable is launching very soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged ASAP. All changes MUST be merged into the release branch by 5pm on Apr-8 to make into the desktop Stable final build cut. Thanks!
,
Apr 8 2016
As of now i am removing 'RBS' label, please feel free to add it back once we got the intended repro case. Thank you!
,
Apr 8 2016
I provided detailed repro instructions in comment #8. The ball is in your court. This is a regression where v8 mis-executes code. This could be a security problem in the right context. Please consider taking it seriously.
,
Apr 8 2016
Jochen, can you help triage this? It seems to be falling through the cracks.
,
Apr 13 2016
Jochen@ : As per the above comment changed the status to assigned, could you please take a look into this.
,
Apr 19 2016
@jochen: Gentle ping. Could you please update this thread. Thank you.
,
Apr 19 2016
I saw the issue and will bisect as soon as I have some free cycles.
,
Apr 19 2016
looked into this, and I cannot reproduce this with the instructions provided in comment #1. kenton@ can you confirm that this still repros for you? Can you make sure that you're always selecting the "top" frame in the console before typing in the JS (https://developer.chrome.com/devtools/docs/tips-and-tricks#console-iframe)?
,
Apr 19 2016
Indeed, with 50.0.2661.75 and with the current state of oasis.sandstorm.io and docs.meteor.com, the problem no longer reproduces using the steps I provided. I'd give it a 50/50 chance between the v8 bug having been fixed independently, and circumstances having changed subtly such that this repro doesn't work anymore even though the bug is still there. But I suppose we'll never know!
,
Apr 20 2016
thanks for checking, I'll close this as WontFix for the time being. If you run into this again, please report back. Also, if you're actually observing another bug, and just this reduced test case doesn't work anymore, feel free to report the other bug as well. |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by cbiesin...@chromium.org
, Mar 22 2016