⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 287812 in webkit


Ignore:
Timestamp:
Jan 8, 2022, 10:21:55 AM (5 years ago)
Author:
commit-queue@webkit.org
Message:

null ptr deref in WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange
​https://bugs.webkit.org/show_bug.cgi?id=234862

Patch by Gabriel Nava Marino <​gnavamarino@apple.com> on 2022-01-08
Reviewed by Darin Adler.

Source/WebCore:

ModifySelectionListLevelCommand::appendSiblingNodeRange loops through nodes assuming
existence of siblings, which is not guaranteed, and can result in nullptr deref. Instead,
check for node existence as part of loop condition, and change raw pointer usage to RefPtr.

This addresses the crash but results in ASSERT(isEndOfParagraph(endOfParagraphToMove))
failing in CompositeEditCommand::moveParagraph. We modify WebCore::findEndOfParagraph
to check for HTMLBRElement nodes to avoid unexpectedly changing the AnchorType.

Test: http/tests/lists/list-new-parent-no-sibling-append.html

  • editing/ModifySelectionListLevel.cpp:

(WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeBefore):
(WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeAfter):
(WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange):

  • editing/VisibleUnits.cpp:

(WebCore::findEndOfParagraph):

LayoutTests:

  • http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
  • http/tests/lists/list-new-parent-no-sibling-append.html: Added.
  • platform/gtk/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
  • platform/win/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
Location:
trunk
Files:
7 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r287810 r287812  
     12022-01-08  Gabriel Nava Marino  <gnavamarino@apple.com>
     2
     3        null ptr deref in WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange
     4        https://bugs.webkit.org/show_bug.cgi?id=234862
     5
     6        Reviewed by Darin Adler.
     7
     8        * http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
     9        * http/tests/lists/list-new-parent-no-sibling-append.html: Added.
     10        * platform/gtk/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
     11        * platform/win/http/tests/lists/list-new-parent-no-sibling-append-expected.txt: Added.
     12
    1132022-01-08  Tyler Wilcock  <tyler_w@apple.com>
    214

  • trunk/Source/WebCore/ChangeLog

    r287811 r287812  
     12022-01-08  Gabriel Nava Marino  <gnavamarino@apple.com>
     2
     3        null ptr deref in WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange
     4        https://bugs.webkit.org/show_bug.cgi?id=234862
     5
     6        Reviewed by Darin Adler.
     7
     8        ModifySelectionListLevelCommand::appendSiblingNodeRange loops through nodes assuming
     9        existence of siblings, which is not guaranteed, and can result in nullptr deref. Instead,
     10        check for node existence as part of loop condition, and change raw pointer usage to RefPtr.
     11
     12        This addresses the crash but results in ASSERT(isEndOfParagraph(endOfParagraphToMove))
     13        failing in CompositeEditCommand::moveParagraph. We modify WebCore::findEndOfParagraph
     14        to check for HTMLBRElement nodes to avoid unexpectedly changing the AnchorType.
     15
     16        Test: http/tests/lists/list-new-parent-no-sibling-append.html
     17
     18        * editing/ModifySelectionListLevel.cpp:
     19        (WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeBefore):
     20        (WebCore::ModifySelectionListLevelCommand::insertSiblingNodeRangeAfter):
     21        (WebCore::ModifySelectionListLevelCommand::appendSiblingNodeRange):
     22        * editing/VisibleUnits.cpp:
     23        (WebCore::findEndOfParagraph):
     24
    1252022-01-08  Tyler Wilcock  <tyler_w@apple.com>
    226

  • trunk/Source/WebCore/editing/ModifySelectionListLevel.cpp

    r283851 r287812  
    9393void ModifySelectionListLevelCommand::insertSiblingNodeRangeBefore(Node* startNode, Node* endNode, Node* refNode)
    9494{
    95     Node* node = startNode;
    96     while (1) {
    97         Node* next = node->nextSibling();
     95    RefPtr node = startNode;
     96    while (node) {
     97        RefPtr next = node->nextSibling();
    9898        removeNode(*node);
    9999        insertNodeBefore(*node, *refNode);
    100100
    101101        if (node == endNode)
    102             break;
     102            return;
    103103
    104104        node = next;
    105105    }
     106    ASSERT_NOT_REACHED();
    106107}
    107108
    108109void ModifySelectionListLevelCommand::insertSiblingNodeRangeAfter(Node* startNode, Node* endNode, Node* refNode)
    109110{
    110     Node* node = startNode;
    111     while (1) {
    112         Node* next = node->nextSibling();
     111    RefPtr node = startNode;
     112    RefPtr refChild = refNode;
     113    while (node) {
     114        RefPtr next = node->nextSibling();
    113115        removeNode(*node);
    114         insertNodeAfter(*node, *refNode);
     116        insertNodeAfter(*node, *refChild);
    115117
    116118        if (node == endNode)
    117             break;
    118 
    119         refNode = node;
     119            return;
     120
     121        refChild = node;
    120122        node = next;
    121123    }
     124    ASSERT_NOT_REACHED();
    122125}
    123126
    124127void ModifySelectionListLevelCommand::appendSiblingNodeRange(Node* startNode, Node* endNode, Element* newParent)
    125128{
    126     Node* node = startNode;
    127     while (1) {
    128         Node* next = node->nextSibling();
     129    RefPtr node = startNode;
     130    while (node) {
     131        RefPtr next = node->nextSibling();
    129132        removeNode(*node);
    130133        appendNode(*node, *newParent);
    131134
    132135        if (node == endNode)
    133             break;
     136            return;
    134137
    135138        node = next;
    136139    }
     140    ASSERT_NOT_REACHED();
    137141}
    138142

  • trunk/Source/WebCore/editing/VisibleUnits.cpp

    r287520 r287812  
    11971197       
    11981198        // FIXME: This is wrong when startNode is a block. We should return a position after the block.
    1199         if (r->isBR() || isBlock(n))
     1199        if (r->isBR() || is<HTMLBRElement>(n) || isBlock(n))
    12001200            break;
    12011201
Note: See TracChangeset for help on using the changeset viewer.