Bug 43354
| Summary: | Potential Null Crasher in ReplaceSelectionCommand | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> |
| Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED INVALID | ||
| Severity: | Normal | CC: | rniwa, tony |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | OS X 10.5 | ||
Eric Seidel (no email)
http://trac.webkit.org/browser/trunk/WebCore/editing/ReplaceSelectionCommand.cpp#L935
935 if ((isListElement(refNode.get()) || (isStyleSpan(refNode.get()) && isListElement(refNode->firstChild())))
936 && blockStart->renderer()->isListItem())
937 refNode = insertAsListItems(refNode, blockStart, insertionPos);
I suspect that unguarded renderer()-> call could crash. Nodes don't have to have renderers.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Eric Seidel (no email)
I'm about to change that line of code a little.
Ryosuke Niwa
+tony since svn annotate indicates he has edited that part of code in two changesets 54413 and 54931.
Tony Chang
We should probably just use isListItem() from htmlediting.h, although I'm surprised that enclosingBlock() can return a node w/o a renderer. Maybe <html> is a block w/o a renderer?
Ryosuke Niwa
Node* blockStart = enclosingBlock(insertionPos.deprecatedNode());
But
bool isBlock(const Node* node)
{
return node && node->renderer() && !node->renderer()->isInline() && !node->renderer()->isRubyText();
}
Element* enclosingBlock(Node* node, EditingBoundaryCrossingRule rule)
{
Node* enclosingNode = enclosingNodeOfType(firstPositionInOrBeforeNode(node), isBlock, rule);
return enclosingNode && enclosingNode->isElementNode() ? toElement(enclosingNode) : 0;
}
So this can never cause a crash.