Skip to content

Commit

Permalink
Merge r246868 - ReplacementFragment should not have script observable…
Browse files Browse the repository at this point in the history
… side effects

https://bugs.webkit.org/show_bug.cgi?id=199147

Reviewed by Wenson Hsieh.

Source/WebCore:

Fixed the bug that ReplacementFragment has script observable side effects.

Use a brand new document for sanitization where the script is disabled for test rendering,
and remove style and script elements as well as event handlers before the test rendering
and the actual pasting.

Test: editing/pasteboard/paste-contents-with-side-effects.html

* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplacementFragment::document): Deleted.
(WebCore::ReplacementFragment::ReplacementFragment): Use createPageForSanitizingWebContent
to create our own document for test rendering. We need to copy over the computed style
from the root editable element (editing host) to respect whitespace treatment, etc...
(WebCore::ReplacementFragment::removeContentsWithSideEffects): Moved from removeHeadContents.
Now removes event handlers and JavaScript URLs.
(WebCore::ReplacementFragment::insertFragmentForTestRendering): Renamed variable names.
(WebCore::ReplaceSelectionCommand::willApplyCommand): Create the plain text and HTML markup
for beforeinput and input events before ReplacementFragment removes contents with side effects.
(WebCore::ReplaceSelectionCommand::ensureReplacementFragment): The removal of head elements
is now done in ReplacementFragment's constructor.

LayoutTests:

Added regression tests.

* editing/pasteboard/paste-contents-with-side-effects-expected.txt: Added.
* editing/pasteboard/paste-contents-with-side-effects.html: Added.
  • Loading branch information
rniwa authored and mcatanzaro committed Aug 4, 2019
1 parent f2119a0 commit 14d995d
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 46 deletions.
12 changes: 12 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
2019-06-26 Ryosuke Niwa <rniwa@webkit.org>

ReplacementFragment should not have script observable side effects
https://bugs.webkit.org/show_bug.cgi?id=199147

Reviewed by Wenson Hsieh.

Added regression tests.

* editing/pasteboard/paste-contents-with-side-effects-expected.txt: Added.
* editing/pasteboard/paste-contents-with-side-effects.html: Added.

2019-06-03 Yusuke Suzuki <ysuzuki@apple.com>

[JSC] JSObject::attemptToInterceptPutByIndexOnHole should use getPrototype instead of getPrototypeDirect
Expand Down
@@ -0,0 +1,22 @@
This tests inserting content with an event handler. WebKit should never execute event handlers.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".



Inserting with load event handler
PASS event.dataTransfer.getData('text/html').includes('onload="') is true
PASS event.dataTransfer.getData('text/html').includes('onmouseover="') is true
PASS didExecute is false

Inserting with script element
PASS event.dataTransfer.getData('text/html').includes('script') is true
PASS didExecute is false

Inserting iframe with a name into plaintext-only
PASS event.dataTransfer.getData("text/html").includes("iframe name=") is true
PASS didExecute is false
PASS successfullyParsed is true

TEST COMPLETE

@@ -0,0 +1,67 @@
<!DOCTYPE html>
<html>
<body>
<script src="../../resources/js-test.js"></script>
<div id="editor" contenteditable></div>
<script>

description('This tests inserting content with an event handler. WebKit should never execute event handlers.');

editor.focus();

function insertHTML(markup) {
editor.textContent = '';
editor.focus();
document.execCommand('insertHTML', false, markup);
}

let didExecute = false;
debug('');
debug('Inserting with load event handler');
editor.addEventListener('beforeinput', () => {
shouldBeTrue(`event.dataTransfer.getData('text/html').includes('onload="')`);
shouldBeTrue(`event.dataTransfer.getData('text/html').includes('onmouseover="')`);
}, {once: true});
insertHTML('<iframe onload="didExecute = true" onmouseover="alert(\'FAIL\')"></iframe>');
shouldBeFalse('didExecute');

didExecute = false;
debug('');
debug('Inserting with script element');
editor.addEventListener('beforeinput', () => {
shouldBeTrue(`event.dataTransfer.getData('text/html').includes('script')`);
}, {once: true});
insertHTML(`<iframe src="data:text/html,<!DOCTYPE html><b>hi</b><script>alert("FAIL")</scr` + 'ipt>"></iframe>');
shouldBeFalse('didExecute');

didExecute = false;
debug('');
debug('Inserting iframe with a name into plaintext-only');
editor.setAttribute('contenteditable', 'plaintext-only');

let i = 0;
function insertObjectElement() {
const object = document.createElement('object');
object.data = 'about:blank';
object.onload = () => {
try {
if (window.open('about:blank', 'named-frame').frameElement.parentNode)
didExecute = true;
} catch (e) { }
if (!didExecute)
insertObjectElement();
}
document.body.appendChild(object);
}
insertObjectElement();
editor.focus();
editor.addEventListener('beforeinput', () => {
shouldBeTrue(`event.dataTransfer.getData("text/html").includes("iframe name=")`);
}, {once: true});
insertHTML(`<iframe name='named-frame'></iframe>`);
shouldBeFalse('didExecute');
didExecute = true;

</script>
</body>
</html>
28 changes: 28 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
2019-06-26 Ryosuke Niwa <rniwa@webkit.org>

ReplacementFragment should not have script observable side effects
https://bugs.webkit.org/show_bug.cgi?id=199147

Reviewed by Wenson Hsieh.

Fixed the bug that ReplacementFragment has script observable side effects.

Use a brand new document for sanitization where the script is disabled for test rendering,
and remove style and script elements as well as event handlers before the test rendering
and the actual pasting.

Test: editing/pasteboard/paste-contents-with-side-effects.html

* editing/ReplaceSelectionCommand.cpp:
(WebCore::ReplacementFragment::document): Deleted.
(WebCore::ReplacementFragment::ReplacementFragment): Use createPageForSanitizingWebContent
to create our own document for test rendering. We need to copy over the computed style
from the root editable element (editing host) to respect whitespace treatment, etc...
(WebCore::ReplacementFragment::removeContentsWithSideEffects): Moved from removeHeadContents.
Now removes event handlers and JavaScript URLs.
(WebCore::ReplacementFragment::insertFragmentForTestRendering): Renamed variable names.
(WebCore::ReplaceSelectionCommand::willApplyCommand): Create the plain text and HTML markup
for beforeinput and input events before ReplacementFragment removes contents with side effects.
(WebCore::ReplaceSelectionCommand::ensureReplacementFragment): The removal of head elements
is now done in ReplacementFragment's constructor.

2019-06-25 Keith Miller <keith_miller@apple.com>

Add didBecomePrototype() calls to global context prototypes
Expand Down
105 changes: 59 additions & 46 deletions Source/WebCore/editing/ReplaceSelectionCommand.cpp
Expand Up @@ -31,6 +31,7 @@
#include "ApplyStyleCommand.h"
#include "BeforeTextInsertedEvent.h"
#include "BreakBlockquoteCommand.h"
#include "CSSComputedStyleDeclaration.h"
#include "CSSStyleDeclaration.h"
#include "DOMWrapperWorld.h"
#include "DataTransfer.h"
Expand All @@ -43,6 +44,7 @@
#include "FrameSelection.h"
#include "HTMLBRElement.h"
#include "HTMLBaseElement.h"
#include "HTMLBodyElement.h"
#include "HTMLInputElement.h"
#include "HTMLLIElement.h"
#include "HTMLLinkElement.h"
Expand All @@ -54,6 +56,7 @@
#include "NodeRenderStyle.h"
#include "RenderInline.h"
#include "RenderText.h"
#include "ScriptElement.h"
#include "SimplifyMarkupCommand.h"
#include "SmartReplace.h"
#include "StyleProperties.h"
Expand All @@ -70,15 +73,13 @@ using namespace HTMLNames;

enum EFragmentType { EmptyFragment, SingleTextNodeFragment, TreeFragment };

static void removeHeadContents(ReplacementFragment&);

// --- ReplacementFragment helper class

class ReplacementFragment {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(ReplacementFragment);
public:
ReplacementFragment(Document&, DocumentFragment*, const VisibleSelection&);
ReplacementFragment(DocumentFragment*, const VisibleSelection&);

DocumentFragment* fragment() { return m_fragment.get(); }

Expand All @@ -94,16 +95,14 @@ class ReplacementFragment {
void removeNodePreservingChildren(Node&);

private:
void removeContentsWithSideEffects();
Ref<HTMLElement> insertFragmentForTestRendering(Node* rootEditableNode);
void removeUnrenderedNodes(Node*);
void restoreAndRemoveTestRenderingNodesToFragment(StyledElement*);
void removeInterchangeNodes(Node*);

void insertNodeBefore(Node&, Node& refNode);

Document& document() { return *m_document; }

RefPtr<Document> m_document;
RefPtr<DocumentFragment> m_fragment;
bool m_hasInterchangeNewlineAtStart;
bool m_hasInterchangeNewlineAtEnd;
Expand Down Expand Up @@ -151,22 +150,23 @@ static Position positionAvoidingPrecedingNodes(Position position)
return position;
}

ReplacementFragment::ReplacementFragment(Document& document, DocumentFragment* fragment, const VisibleSelection& selection)
: m_document(&document)
, m_fragment(fragment)
ReplacementFragment::ReplacementFragment(DocumentFragment* fragment, const VisibleSelection& selection)
: m_fragment(fragment)
, m_hasInterchangeNewlineAtStart(false)
, m_hasInterchangeNewlineAtEnd(false)
{
if (!m_fragment)
return;
if (!m_fragment->firstChild())
return;


removeContentsWithSideEffects();

RefPtr<Element> editableRoot = selection.rootEditableElement();
ASSERT(editableRoot);
if (!editableRoot)
return;

auto* shadowHost = editableRoot->shadowHost();
if (!editableRoot->attributeEventListener(eventNames().webkitBeforeTextInsertedEvent, mainThreadNormalWorld())
&& !(shadowHost && shadowHost->renderer() && shadowHost->renderer()->isTextControl())
Expand All @@ -175,7 +175,14 @@ ReplacementFragment::ReplacementFragment(Document& document, DocumentFragment* f
return;
}

RefPtr<StyledElement> holder = insertFragmentForTestRendering(editableRoot.get());
auto page = createPageForSanitizingWebContent();
Document* stagingDocument = page->mainFrame().document();
ASSERT(stagingDocument->body());

ComputedStyleExtractor computedStyleOfEditableRoot(editableRoot.get());
stagingDocument->body()->setAttributeWithoutSynchronization(styleAttr, computedStyleOfEditableRoot.copyProperties()->asText());

RefPtr<StyledElement> holder = insertFragmentForTestRendering(stagingDocument->body());
if (!holder) {
removeInterchangeNodes(m_fragment.get());
return;
Expand All @@ -202,13 +209,44 @@ ReplacementFragment::ReplacementFragment(Document& document, DocumentFragment* f
if (!m_fragment->firstChild())
return;

holder = insertFragmentForTestRendering(editableRoot.get());
holder = insertFragmentForTestRendering(stagingDocument->body());
removeInterchangeNodes(holder.get());
removeUnrenderedNodes(holder.get());
restoreAndRemoveTestRenderingNodesToFragment(holder.get());
}
}

void ReplacementFragment::removeContentsWithSideEffects()
{
Vector<Ref<Element>> elementsToRemove;
Vector<std::pair<Ref<Element>, QualifiedName>> attributesToRemove;

auto it = descendantsOfType<Element>(*m_fragment).begin();
auto end = descendantsOfType<Element>(*m_fragment).end();
while (it != end) {
auto element = makeRef(*it);
if (isScriptElement(element) || (is<HTMLStyleElement>(element) && element->getAttribute(classAttr) != WebKitMSOListQuirksStyle)
|| is<HTMLBaseElement>(element) || is<HTMLLinkElement>(element) || is<HTMLMetaElement>(element) || is<HTMLTitleElement>(element)) {
elementsToRemove.append(WTFMove(element));
it.traverseNextSkippingChildren();
continue;
}
if (element->hasAttributes()) {
for (auto& attribute : element->attributesIterator()) {
if (element->isEventHandlerAttribute(attribute) || element->isJavaScriptURLAttribute(attribute))
attributesToRemove.append({ element.copyRef(), attribute.name() });
}
}
++it;
}

for (auto& element : elementsToRemove)
removeNode(WTFMove(element));

for (auto& item : attributesToRemove)
item.first->removeAttribute(item.second);
}

bool ReplacementFragment::isEmpty() const
{
return (!m_fragment || !m_fragment->firstChild()) && !m_hasInterchangeNewlineAtStart && !m_hasInterchangeNewlineAtEnd;
Expand Down Expand Up @@ -252,13 +290,14 @@ void ReplacementFragment::insertNodeBefore(Node& node, Node& refNode)
parent->insertBefore(node, &refNode);
}

Ref<HTMLElement> ReplacementFragment::insertFragmentForTestRendering(Node* rootEditableElement)
Ref<HTMLElement> ReplacementFragment::insertFragmentForTestRendering(Node* rootNode)
{
auto holder = createDefaultParagraphElement(document());
auto document = makeRef(rootNode->document());
auto holder = createDefaultParagraphElement(document.get());

holder->appendChild(*m_fragment);
rootEditableElement->appendChild(holder);
document().updateLayoutIgnorePendingStylesheets();
rootNode->appendChild(holder);
document->updateLayoutIgnorePendingStylesheets();

return holder;
}
Expand Down Expand Up @@ -726,29 +765,6 @@ VisiblePosition ReplaceSelectionCommand::positionAtStartOfInsertedContent() cons
return m_startOfInsertedContent;
}

static void removeHeadContents(ReplacementFragment& fragment)
{
if (fragment.isEmpty())
return;

Vector<Element*> toRemove;

auto it = descendantsOfType<Element>(*fragment.fragment()).begin();
auto end = descendantsOfType<Element>(*fragment.fragment()).end();
while (it != end) {
if (is<HTMLBaseElement>(*it) || is<HTMLLinkElement>(*it) || is<HTMLMetaElement>(*it) || is<HTMLTitleElement>(*it)
|| (is<HTMLStyleElement>(*it) && it->getAttribute(classAttr) != WebKitMSOListQuirksStyle)) {
toRemove.append(&*it);
it.traverseNextSkippingChildren();
continue;
}
++it;
}

for (auto& element : toRemove)
fragment.removeNode(*element);
}

// Remove style spans before insertion if they are unnecessary. It's faster because we'll
// avoid doing a layout.
static bool handleStyleSpansBeforeInsertion(ReplacementFragment& fragment, const Position& insertionPos)
Expand Down Expand Up @@ -919,9 +935,9 @@ inline Node* nodeToSplitToAvoidPastingIntoInlineNodesWithStyle(const Position& i

bool ReplaceSelectionCommand::willApplyCommand()
{
ensureReplacementFragment();
m_documentFragmentPlainText = m_documentFragment->textContent();
m_documentFragmentHTMLMarkup = serializeFragment(*m_documentFragment, SerializedNodes::SubtreeIncludingNode);
ensureReplacementFragment();
return CompositeEditCommand::willApplyCommand();
}

Expand Down Expand Up @@ -1544,11 +1560,8 @@ void ReplaceSelectionCommand::updateNodesInserted(Node *node)

ReplacementFragment* ReplaceSelectionCommand::ensureReplacementFragment()
{
if (!m_replacementFragment) {
m_replacementFragment = std::make_unique<ReplacementFragment>(document(), m_documentFragment.get(), endingSelection());
removeHeadContents(*m_replacementFragment);
}

if (!m_replacementFragment)
m_replacementFragment = std::make_unique<ReplacementFragment>(m_documentFragment.get(), endingSelection());
return m_replacementFragment.get();
}

Expand Down

0 comments on commit 14d995d

Please sign in to comment.