Skip to content

Commit

Permalink
Remove url change for about:srcdoc document.write()
Browse files Browse the repository at this point in the history
This CL removes the ability for either srcdoc iframes
to change another frame's url via document.write. That ability has
been shown to allow a mainframe to acquire a srcdoc url, which
causes problems in the codebase.

This CL also adds a test to make sure the document.write doesn't
give a srcdoc url to a mainframe, and modifies an existing test to
cover the about:blank url-writing case.

Bug: 1478463
Change-Id: I85895c1e16df6734c9e351ea798fd1800e0a4df7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4856149
Reviewed-by: Dominic Farolino <[email protected]>
Commit-Queue: James Maclean <[email protected]>
Reviewed-by: Charlie Reis <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1205943}
  • Loading branch information
W. James MacLean authored and Lightning00Blade committed Dec 11, 2023
1 parent d2ccc52 commit 20008a4
Showing 1 changed file with 27 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,19 @@

window.start = childDocument => {
const grandchildDocument =
childDocument.querySelector('iframe').contentDocument;
childDocument.getElementById('foo').contentDocument;

test(t => {
// Clean up the iframe so that the dangling document.open() doesn't cause a
// harness timeout.
t.add_cleanup(() => {
document.querySelector('iframe').remove();
});

assert_equals(childDocument.URL, 'about:srcdoc',
assert_equals(childDocument.URL, 'about:blank',
'Child document starting URL');
assert_equals(grandchildDocument.URL, 'about:blank',
assert_equals(grandchildDocument.URL, 'about:srcdoc',
'Grandchild document starting URL');
const originalChildBaseURL = childDocument.baseURI;

grandchildDocument.open("", "");
// Verify that the document.open() trick worked: the grandchild should now
// have the same url as the child, and have inherited the child's base url.
assert_equals(grandchildDocument.URL, 'about:srcdoc',
assert_equals(grandchildDocument.URL, 'about:blank',
'Grandchild document after document.open() trick');
assert_equals(grandchildDocument.baseURI, originalChildBaseURL,
'Grandchild base URL must match child base URL');
Expand All @@ -41,7 +35,7 @@
// Verify that changing the child's base url succeeded and did not affect
// the grandchild's base url.
const newChildBaseURL = childDocument.baseURI;
assert_equals(grandchildDocument.URL, 'about:srcdoc',
assert_equals(grandchildDocument.URL, 'about:blank',
'Grandchild document after child gets new base URL');
assert_not_equals(newChildBaseURL, originalChildBaseURL,
'Child base URL must change');
Expand All @@ -53,5 +47,25 @@
};
</script>

<iframe srcdoc="<iframe></iframe><script>parent.start(document)</script>">
</iframe>
<iframe src='about:blank'></iframe>

<script>
window.onload = () => {
let subframe_doc = document.querySelector('iframe').contentDocument;
subframe_doc.body.innerHTML = '<iframe srcdoc="foo" id="foo"></iframe>';
promise_test((test) => {
return new Promise(async resolve => {
// We need a timeout since the srcdoc frame takes time to setup and
// doesn't fire a loadstop.
test.step_timeout(resolve, 100);
}).then(() => {
assert_equals(
subframe_doc.getElementById('foo').contentDocument.URL,
'about:srcdoc');
let script = subframe_doc.createElement('script');
script.innerHTML = 'parent.start(document);';
subframe_doc.body.appendChild(script);
});
}, "wrapper promise test for timeout.");
};
</script>

0 comments on commit 20008a4

Please sign in to comment.