Crash in WebCore::ApplyStyleCommand::applyBlockStyle

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Created attachment 70796 [details]
demo

We don't check nullity of start/end in applyBlockStyle and we hit an assert inside highestAncestor.

[Ryosuke Niwa]

Oops, sorry.  This isn't really a security bug.

[Ryosuke Niwa]

http://crbug.com/58832

[Ryosuke Niwa]

Created attachment 70834 [details]
fixes the crash

[Darin Adler]

Comment on attachment 70834 [details]
fixes the crash

View in context: https://bugs.webkit.org/attachment.cgi?id=70834&action=review

> WebCore/editing/ApplyStyleCommand.cpp:648
> +    if (visibleStart.isNull() || visibleStart.isOrphan() || visibleEnd.isNull() || visibleEnd.isOrphan())
> +        return;

Makes me wish isOrphan returned true for null. There seem to be very few cases where we check isOrphan and want to handle isNull differently.

[Ryosuke Niwa]

(In reply to comment #4)
> (From update of attachment 70834 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70834&action=review
> 
> > WebCore/editing/ApplyStyleCommand.cpp:648
> > +    if (visibleStart.isNull() || visibleStart.isOrphan() || visibleEnd.isNull() || visibleEnd.isOrphan())
> > +        return;
> 
> Makes me wish isOrphan returned true for null. There seem to be very few cases where we check isOrphan and want to handle isNull differently.

We could do that given the following result for isOrphan but we probably want to rename isOrphan to isOrphanOrNull if we made this change.

VisiblePosition.h:     bool isOrphan() const { return m_deepPosition.isOrphan(); }
VisiblePosition.h:     bool isOrphan() const { return m_deepPosition.isOrphan(); }
InsertListCommand.cpp:                 if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
InsertListCommand.cpp:                 if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
Position.h:     bool isOrphan() const { return m_anchorNode && !m_anchorNode->inDocument(); }
ApplyStyleCommand.cpp:     if (visibleStart.isNull() || visibleStart.isOrphan() || visibleEnd.isNull() || visibleEnd.isOrphan())
ApplyStyleCommand.cpp:     if (visibleStart.isNull() || visibleStart.isOrphan() || visibleEnd.isNull() || visibleEnd.isOrphan())
ApplyStyleCommand.cpp:             if (nextParagraphStart.isOrphan())
ApplyStyleCommand.cpp:     Position s = start.isNull() || start.isOrphan() ? pushDownStart : start;
ApplyStyleCommand.cpp:     Position e = end.isNull() || end.isOrphan() ? pushDownEnd : end;
Editor.cpp:     if (newSelection.start().isOrphan() || newSelection.end().isOrphan())
Editor.cpp:     if (newSelection.start().isOrphan() || newSelection.end().isOrphan())
VisibleSelection.h:     bool isNonOrphanedRange() const { return isRange() && !start().isOrphan() && !end().isOrphan(); }
VisibleSelection.h:     bool isNonOrphanedRange() const { return isRange() && !start().isOrphan() && !end().isOrphan(); }
VisibleSelection.h:     bool isNonOrphanedCaretOrRange() const { return isCaretOrRange() && !start().isOrphan() && !end().isOrphan(); }
VisibleSelection.h:     bool isNonOrphanedCaretOrRange() const { return isCaretOrRange() && !start().isOrphan() && !end().isOrphan(); }

[Ryosuke Niwa]

Committed r69865: <http://trac.webkit.org/changeset/69865>

[WebKit Review Bot]

http://trac.webkit.org/changeset/69865 might have broken Qt Linux Release
The following tests are not passing:
http/tests/security/mixedContent/insecure-css-in-main-frame.html