Skip to content
Permalink
Browse files
Correct nullptr deref in selection handling.
https://bugs.webkit.org/show_bug.cgi?id=178189
<rdar://problem/33833012>

Reviewed by Ryosuke Niwa.

The VisibleSelection::toNormalizedRange returns nullptr for certain conditions (e.g., 'isNone'
and 'isOrphaned' cases). It's possible to crash the WebProcess by executing a code path with
an orphaned selection range.

The return value of 'toNormalizedRange' is checked for nullptr in many places, but not everywhere.
This patch adds those missing nullptr checks.

* accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
(-[WebAccessibilityObjectWrapper textMarkerRangeForSelection]):
* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::makeStylingElementsDirectChildrenOfEditableRootToPreventStyleLoss):
* editing/EditingStyle.cpp:
(WebCore::EditingStyle::styleAtSelectionStart):
* editing/Editor.cpp:
(WebCore::Editor::misspelledWordAtCaretOrRange const):
* page/DOMSelection.cpp:
(WebCore::DOMSelection::containsNode const):
* page/DragController.cpp:
(WebCore::DragController::concludeEditDrag):


Canonical link: https://commits.webkit.org/194455@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223228 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
brentfulgham committed Oct 12, 2017
1 parent 3a70f4c commit 821eddc30d13736ce3bd1ab884c39fd967cb2bf9
Showing 7 changed files with 42 additions and 4 deletions.
@@ -1,3 +1,31 @@
2017-10-11 Brent Fulgham <bfulgham@apple.com>

Correct nullptr deref in selection handling.
https://bugs.webkit.org/show_bug.cgi?id=178189
<rdar://problem/33833012>

Reviewed by Ryosuke Niwa.

The VisibleSelection::toNormalizedRange returns nullptr for certain conditions (e.g., 'isNone'
and 'isOrphaned' cases). It's possible to crash the WebProcess by executing a code path with
an orphaned selection range.

The return value of 'toNormalizedRange' is checked for nullptr in many places, but not everywhere.
This patch adds those missing nullptr checks.

* accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
(-[WebAccessibilityObjectWrapper textMarkerRangeForSelection]):
* editing/DeleteSelectionCommand.cpp:
(WebCore::DeleteSelectionCommand::makeStylingElementsDirectChildrenOfEditableRootToPreventStyleLoss):
* editing/EditingStyle.cpp:
(WebCore::EditingStyle::styleAtSelectionStart):
* editing/Editor.cpp:
(WebCore::Editor::misspelledWordAtCaretOrRange const):
* page/DOMSelection.cpp:
(WebCore::DOMSelection::containsNode const):
* page/DragController.cpp:
(WebCore::DragController::concludeEditDrag):

2017-10-11 Ryan Haddad <ryanhaddad@apple.com>

Unreviewed, rolling out r223215.
@@ -2449,6 +2449,9 @@ - (NSArray *)textMarkerRangeForSelection
return nil;

RefPtr<Range> range = selection.toNormalizedRange();
if (!range)
return nil;

CharacterOffset start = cache->startOrEndCharacterOffsetForRange(range, true);
CharacterOffset end = cache->startOrEndCharacterOffsetForRange(range, false);

@@ -461,7 +461,7 @@ void DeleteSelectionCommand::deleteTextFromNode(Text& node, unsigned offset, uns
void DeleteSelectionCommand::makeStylingElementsDirectChildrenOfEditableRootToPreventStyleLoss()
{
RefPtr<Range> range = m_selectionToDelete.toNormalizedRange();
RefPtr<Node> node = range->firstNode();
RefPtr<Node> node = range ? range->firstNode() : nullptr;
while (node && node != range->pastLastNode()) {
RefPtr<Node> nextNode = NodeTraversal::next(*node);
if ((is<HTMLStyleElement>(*node) && !downcast<HTMLStyleElement>(*node).hasAttributeWithoutSynchronization(scopedAttr)) || is<HTMLLinkElement>(*node)) {
@@ -1445,9 +1445,10 @@ RefPtr<EditingStyle> EditingStyle::styleAtSelectionStart(const VisibleSelection&
// Also, if the selection is a range, ignore the background color at the start of selection,
// and find the background color of the common ancestor.
if (shouldUseBackgroundColorInEffect && (selection.isRange() || hasTransparentBackgroundColor(style->m_mutableStyle.get()))) {
RefPtr<Range> range(selection.toNormalizedRange());
if (auto value = backgroundColorInEffect(range->commonAncestorContainer()))
style->setProperty(CSSPropertyBackgroundColor, value->cssText());
if (auto range = selection.toNormalizedRange()) {
if (auto value = backgroundColorInEffect(range->commonAncestorContainer()))
style->setProperty(CSSPropertyBackgroundColor, value->cssText());
}
}

return WTFMove(style);
@@ -2180,6 +2180,8 @@ String Editor::misspelledWordAtCaretOrRange(Node* clickedNode) const
VisibleSelection wordSelection(selection.base());
wordSelection.expandUsingGranularity(WordGranularity);
RefPtr<Range> wordRange = wordSelection.toNormalizedRange();
if (!wordRange)
return String();

// In compliance with GTK+ applications, additionally allow to provide suggestions when the current
// selection exactly match the word selection.
@@ -393,6 +393,8 @@ bool DOMSelection::containsNode(Node& node, bool allowPartial) const

Ref<Node> protectedNode(node);
auto selectedRange = selection.selection().toNormalizedRange();
if (!selectedRange)
return false;

ContainerNode* parentNode = node.parentNode();
if (!parentNode || !parentNode->isConnected())
@@ -536,6 +536,8 @@ bool DragController::concludeEditDrag(const DragData& dragData)
if (!color.isValid())
return false;
RefPtr<Range> innerRange = innerFrame->selection().toNormalizedRange();
if (!innerRange)
return false;
RefPtr<MutableStyleProperties> style = MutableStyleProperties::create();
style->setProperty(CSSPropertyColor, color.serialized(), false);
if (!innerFrame->editor().shouldApplyStyle(style.get(), innerRange.get()))

0 comments on commit 821eddc

Please sign in to comment.