[Webkit-unassigned] [Bug 109220] [Chromium] Fix use after free in ContextMenuClientImpl.cpp

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Rouslan Solomakhin <rouslan+webkit at chromium.org>  2013-02-08 09:39:39 PST ---
The freeing stack trace starts at respondToChangedSelection(). I suspect that the original source is "selectedFrame->selection()->setSelection(selection, WordGranularity)" call in  selectMisspellingAsync(). This call is between getting the markers and using these markers. Therefore, the fix would work because it saves the marker before the potential deletion of the marker. Here is the current code with annotations:

    markers.append(selectedFrame->document()->markers()->markersInRange(selectionRange.get(), DocumentMarker::Spelling | DocumentMarker::Grammar));  // <-- Retrieve the marker pointers.
    if (markers.size() != 1)
        return String();

    // Cloning a range fails only for invalid ranges.
    RefPtr<Range> markerRange = selectionRange->cloneRange(ASSERT_NO_EXCEPTION);
    markerRange->setStart(markerRange->startContainer(), markers[0]->startOffset());
    markerRange->setEnd(markerRange->endContainer(), markers[0]->endOffset());
    if (selection.isCaret()) {
        selection = VisibleSelection(markerRange.get());
        selectedFrame->selection()->setSelection(selection, WordGranularity);  // <-- Potentially free the marker pointers.
        selectionRange = selection.toNormalizedRange();
    }
    ...
    if (markers.size() == 1 && markers[0]->description().length()) {  // <-- Use the potentially freed marker pointers.
    ...
    }

Here is the patched code with annotations:

    Vector<DocumentMarker*> markers = selectedFrame->document()->markers()->markersInRange(selectionRange.get(), DocumentMarker::Spelling | DocumentMarker::Grammar);  // <-- Retrieve the marker pointers.
    if (markers.size() != 1)
        return String();
    marker = *markers[0];  // <-- Save a copy of the marker data locally.

    // Cloning a range fails only for invalid ranges.
    RefPtr<Range> markerRange = selectionRange->cloneRange(ASSERT_NO_EXCEPTION);
    markerRange->setStart(markerRange->startContainer(), marker.startOffset());
    markerRange->setEnd(markerRange->endContainer(), marker.endOffset());
    if (selection.isCaret()) {
        selection = VisibleSelection(markerRange.get());
        selectedFrame->selection()->setSelection(selection, WordGranularity);  // <-- Potentially free the marker pointers.
        selectionRange = selection.toNormalizedRange();
    }
    ...
    if (marker.description().length()) {  // <-- Use the copy of the local marker data instead of the potentially freed marker pointers.
    ...
    }

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.