162877 2016-10-03 13:16:13 -0700 Avoid null dereference when changing focus in design mode. 2016-10-03 14:39:24 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build All All RESOLVED FIXED P2 Normal --- 1 bfulgham bfulgham bfulgham cdumez oldest_to_newest 1235873 0 bfulgham 2016-10-03 13:16:13 -0700 A malformed web page consisting of nested iframes can trigger a null dereference when changing focus in design mode. In this scenario, the DOM Window's m_frame member is set to null. This null value is used without checking to refocus the document view on the now non-existant frame. 1235893 1 290515 bfulgham 2016-10-03 13:51:55 -0700 Created attachment 290515 Patch 1235897 2 290515 cdumez 2016-10-03 13:59:39 -0700 Comment on attachment 290515 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290515&action=review r=me with comments on the test. > LayoutTests/fast/frames/iframe-focus-crash.html:4 > + if (window.testRunner) { nit: unnecessary curly brackets. > LayoutTests/fast/frames/resources/iframe-focus-crash.html:5 > + document.designMode='on'; The indentation is weird / wrong in this block. > LayoutTests/fast/frames/resources/iframe-focus-crash.html:6 > + window.parent.setTimeout(function focusTest() { We do not need to name this function. > LayoutTests/fast/frames/resources/iframe-focus-crash.html:8 > + }, 60); Why 60, wouldn't 0 work? > LayoutTests/fast/frames/resources/iframe-focus-crash.html:12 > + <iframe src='iframe-focus-crash.html'></iframe> Eh, is this a recursive iframe? Could we do without the recursiveness? 1235902 3 290515 bfulgham 2016-10-03 14:18:11 -0700 Comment on attachment 290515 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290515&action=review >> LayoutTests/fast/frames/resources/iframe-focus-crash.html:5 >> + document.designMode='on'; > > The indentation is weird / wrong in this block. Strange. I wonder why webkit-patch didn't complain? It also seems fine in my text editor. I wonder if this is an artifact of the review tool? >> LayoutTests/fast/frames/resources/iframe-focus-crash.html:6 >> + window.parent.setTimeout(function focusTest() { > > We do not need to name this function. OK! >> LayoutTests/fast/frames/resources/iframe-focus-crash.html:8 >> + }, 60); > > Why 60, wouldn't 0 work? Yep -- it will, and I'll change it. >> LayoutTests/fast/frames/resources/iframe-focus-crash.html:12 >> + <iframe src='iframe-focus-crash.html'></iframe> > > Eh, is this a recursive iframe? Could we do without the recursiveness? Unfortunately it is needed to reproduce the crash. 1235908 4 290515 cdumez 2016-10-03 14:26:35 -0700 Comment on attachment 290515 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290515&action=review >>> LayoutTests/fast/frames/resources/iframe-focus-crash.html:5 >>> + document.designMode='on'; >> >> The indentation is weird / wrong in this block. > > Strange. I wonder why webkit-patch didn't complain? It also seems fine in my text editor. I wonder if this is an artifact of the review tool? The reason is that you are mixing tabs and spaces. Please fix. 1235924 5 bfulgham 2016-10-03 14:39:13 -0700 Committed r206751: <http://trac.webkit.org/changeset/206751> 1235925 6 bfulgham 2016-10-03 14:39:24 -0700 (In reply to comment #4) > > Strange. I wonder why webkit-patch didn't complain? It also seems fine in my text editor. I wonder if this is an artifact of the review tool? > > The reason is that you are mixing tabs and spaces. Please fix. Fixed. 290515 2016-10-03 13:51:55 -0700 2016-10-03 13:59:39 -0700 Patch bug-162877-20161003134846.patch text/plain 4307 bfulgham SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDIwNjc0NCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIxIEBACisyMDE2LTEwLTAzICBCcmVudCBG dWxnaGFtICA8YmZ1bGdoYW1AYXBwbGUuY29tPgorCisgICAgICAgIEF2b2lkIG51bGwgZGVyZWZl cmVuY2Ugd2hlbiBjaGFuZ2luZyBmb2N1cyBpbiBkZXNpZ24gbW9kZS4KKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2Mjg3NworICAgICAgICA8cmRhcjov L3Byb2JsZW0vMjgwNjEyNjE+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgVGhlIGJhcmUgbV9mcmFtZSBwb2ludGVyIGluIERPTVdpbmRvdyBjYW4gYmUg Y2xlYXJlZCB3aGVuIHNldHRpbmcgZm9jdXMgdG8gYSBuZXcgZWxlbWVudC4gQ2hlY2sKKyAgICAg ICAgdGhhdCB0aGUgbV9mcmFtZSBwb2ludGVyIGlzIG5vbi1udWxsIGJlZm9yZSB1c2luZyBpdCBh ZnRlciBjYWxsaW5nIGEgcm91dGluZSB0aGF0IGNvdWxkCisgICAgICAgIGNsZWFyIHRoZSBwb2lu dGVyIHZhbHVlLgorCisgICAgICAgIFRlc3Q6IGZhc3QvZnJhbWVzL2lmcmFtZS1mb2N1cy1jcmFz aC5odG1sCisKKyAgICAgICAgKiBwYWdlL0RPTVdpbmRvdy5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpET01XaW5kb3c6OmZvY3VzKTogQ2hlY2sgdGhhdCB0aGUgcG9pbnRlciBpcyBzdGlsbCBub24t bnVsbCBhZnRlciBzZXR0aW5nIHRoZQorICAgICAgICBjdXJyZW50IGZvY3VzZWQgZWxlbWVudCB0 byBudWxscHRyLgorCiAyMDE2LTEwLTAzICBBbmR5IEVzdGVzICA8YWVzdGVzQGFwcGxlLmNvbT4K IAogICAgICAgICBBU1NFUlRJT04gRkFJTEVEOiB1cmwuY29udGFpbnNPbmx5QVNDSUkoKSBpbiBX ZWJDb3JlOjpjaGVja0VuY29kZWRTdHJpbmcoKSB3aGVuIHBhcnNpbmcgYW4gaW52YWxpZCBDU1Mg Y3Vyc29yIFVSTApJbmRleDogU291cmNlL1dlYkNvcmUvcGFnZS9ET01XaW5kb3cuY3BwCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3BhZ2UvRE9NV2luZG93LmNwcAkocmV2aXNpb24gMjA2 NjYxKQorKysgU291cmNlL1dlYkNvcmUvcGFnZS9ET01XaW5kb3cuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0xMDA1LDcgKzEwMDUsOSBAQCB2b2lkIERPTVdpbmRvdzo6Zm9jdXMoYm9vbCBhbGxvd0Zv Y3VzKQogICAgIGlmIChmb2N1c2VkRnJhbWUgJiYgZm9jdXNlZEZyYW1lICE9IG1fZnJhbWUpCiAg ICAgICAgIGZvY3VzZWRGcmFtZS0+ZG9jdW1lbnQoKS0+c2V0Rm9jdXNlZEVsZW1lbnQobnVsbHB0 cik7CiAKLSAgICBtX2ZyYW1lLT5ldmVudEhhbmRsZXIoKS5mb2N1c0RvY3VtZW50VmlldygpOwor ICAgIC8vIHNldEZvY3VzZWRFbGVtZW50IG1heSBjbGVhciBtX2ZyYW1lLCBzbyByZWNoZWNrIGJl Zm9yZSB1c2luZyBpdC4KKyAgICBpZiAobV9mcmFtZSkKKyAgICAgICAgbV9mcmFtZS0+ZXZlbnRI YW5kbGVyKCkuZm9jdXNEb2N1bWVudFZpZXcoKTsKIH0KIAogdm9pZCBET01XaW5kb3c6OmJsdXIo KQpJbmRleDogTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3Rz L0NoYW5nZUxvZwkocmV2aXNpb24gMjA2NjYxKQorKysgTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCSh3 b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTYtMTAtMDMgIEJyZW50IEZ1bGdoYW0g IDxiZnVsZ2hhbUBhcHBsZS5jb20+CisKKyAgICAgICAgQXZvaWQgbnVsbCBkZXJlZmVyZW5jZSB3 aGVuIGNoYW5naW5nIGZvY3VzIGluIGRlc2lnbiBtb2RlLgorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTYyODc3CisgICAgICAgIDxyZGFyOi8vcHJvYmxl bS8yODA2MTI2MT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIGZhc3QvZnJhbWVzL2lmcmFtZS1mb2N1cy1jcmFzaC1leHBlY3RlZC50eHQ6IEFkZGVk LgorICAgICAgICAqIGZhc3QvZnJhbWVzL2lmcmFtZS1mb2N1cy1jcmFzaC5odG1sOiBBZGRlZC4K KyAgICAgICAgKiBmYXN0L2ZyYW1lcy9yZXNvdXJjZXMvaWZyYW1lLWZvY3VzLWNyYXNoLmh0bWw6 IEFkZGVkLgorCiAyMDE2LTA5LTMwICBSeWFuIEhhZGRhZCAgPHJ5YW5oYWRkYWRAYXBwbGUuY29t PgogCiAgICAgICAgIFJlYmFzZWxpbmUganMvZG9tL3N0YWNrLXRyYWNlLmh0bWwgYWZ0ZXIgcjIw NjY1NC4KSW5kZXg6IExheW91dFRlc3RzL2Zhc3QvZnJhbWVzL2lmcmFtZS1mb2N1cy1jcmFzaC1l eHBlY3RlZC50eHQKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvZmFzdC9mcmFtZXMvaWZyYW1l LWZvY3VzLWNyYXNoLWV4cGVjdGVkLnR4dAkobm9uZXhpc3RlbnQpCisrKyBMYXlvdXRUZXN0cy9m YXN0L2ZyYW1lcy9pZnJhbWUtZm9jdXMtY3Jhc2gtZXhwZWN0ZWQudHh0CSh3b3JraW5nIGNvcHkp CkBAIC0wLDAgKzEsMiBAQAorVGhpcyB0ZXN0cyB0aGF0IHNldHRpbmcgZm9jdXMgdG8gYSByZW1v dmVkIGZyYW1lIGRvZXMgbm90IGNhdXNlIGEgY3Jhc2guIFRoZSB0ZXN0IHBhc3NlcyBpZiBpdCBk b2VzIG5vdCBjcmFzaC4KKwpJbmRleDogTGF5b3V0VGVzdHMvZmFzdC9mcmFtZXMvaWZyYW1lLWZv Y3VzLWNyYXNoLmh0bWwKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvZmFzdC9mcmFtZXMvaWZy YW1lLWZvY3VzLWNyYXNoLmh0bWwJKG5vbmV4aXN0ZW50KQorKysgTGF5b3V0VGVzdHMvZmFzdC9m cmFtZXMvaWZyYW1lLWZvY3VzLWNyYXNoLmh0bWwJKHdvcmtpbmcgY29weSkKQEAgLTAsMCArMSwx MyBAQAorPGh0bWw+CisgICAgPGhlYWQ+CisgICAgICAgIDxzY3JpcHQ+CisgICAgICAgIGlmICh3 aW5kb3cudGVzdFJ1bm5lcikgeworICAgICAgICAgICAgdGVzdFJ1bm5lci5kdW1wQXNUZXh0KHRy dWUpOworICAgICAgICB9CisgICAgICAgIDwvc2NyaXB0PgorICAgIDwvaGVhZD4KKyAgICA8Ym9k eT4KKyAgICAgICAgPGRpdj5UaGlzIHRlc3RzIHRoYXQgc2V0dGluZyBmb2N1cyB0byBhIHJlbW92 ZWQgZnJhbWUgZG9lcyBub3QgY2F1c2UgYSBjcmFzaC4gVGhlIHRlc3QgcGFzc2VzIGlmIGl0IGRv ZXMgbm90IGNyYXNoLjwvZGl2PgorICAgICAgICA8aWZyYW1lIHNyYz0ncmVzb3VyY2VzL2lmcmFt ZS1mb2N1cy1jcmFzaC5odG1sJz48L2lmcmFtZT4KKyAgICA8L2JvZHk+Cis8L2h0bWw+ClwgTm8g bmV3bGluZSBhdCBlbmQgb2YgZmlsZQpJbmRleDogTGF5b3V0VGVzdHMvZmFzdC9mcmFtZXMvcmVz b3VyY2VzL2lmcmFtZS1mb2N1cy1jcmFzaC5odG1sCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3Rz L2Zhc3QvZnJhbWVzL3Jlc291cmNlcy9pZnJhbWUtZm9jdXMtY3Jhc2guaHRtbAkobm9uZXhpc3Rl bnQpCisrKyBMYXlvdXRUZXN0cy9mYXN0L2ZyYW1lcy9yZXNvdXJjZXMvaWZyYW1lLWZvY3VzLWNy YXNoLmh0bWwJKHdvcmtpbmcgY29weSkKQEAgLTAsMCArMSwxNSBAQAorPGh0bWw+CisgICAgPGJv ZHkgb25sb2FkPSdydW5UZXN0KCknPiAKKyAgICAgICAgPHNjcmlwdD4KKyAgICAgICAgZnVuY3Rp b24gcnVuVGVzdCgpIHsKKwkJICAgIGRvY3VtZW50LmRlc2lnbk1vZGU9J29uJzsKKwkJICAgIHdp bmRvdy5wYXJlbnQuc2V0VGltZW91dChmdW5jdGlvbiBmb2N1c1Rlc3QoKSB7CisgICAgICAgICAg ICAgICAgd2luZG93LmZvY3VzKCk7CisgICAgICAgICAgICB9LCA2MCk7CisJCSAgICB3aW5kb3cu Zm9jdXMoKTsKKyAgICAgICAgfQorICAgICAgICA8L3NjcmlwdD4KKwkgICAgPGlmcmFtZSBzcmM9 J2lmcmFtZS1mb2N1cy1jcmFzaC5odG1sJz48L2lmcmFtZT4KKwkgICAgPGh0bWwgb25mb2N1c291 dD0id2luZG93LmRvY3VtZW50LndyaXRlbG4oKTsiPjwvaHRtbD4KKyAgICA8L2JvZHk+Cis8L2h0 bWw+ClwgTm8gbmV3bGluZSBhdCBlbmQgb2YgZmlsZQo=