Closed Bug 848895 Opened 7 years ago Closed 7 years ago

Hold strong refs to editrules when calling into it, and always null-check the editor in editrules code

Categories

(Core :: DOM: Editor, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox21 --- wontfix
firefox22 --- fixed
firefox23 --- fixed
firefox-esr17 --- wontfix
b2g18 --- wontfix

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

(Keywords: sec-want, Whiteboard: [adv-main22-])

Attachments

(2 files, 1 obsolete file)

See bug 848644 for discussion.
Attached patch Partial part 2 (obsolete) — Splinter Review
Up to about line 2878 of nsHTMLEditRules.cpp so far.  Lots more to go
Whiteboard: [need to finish up patch]
Marking sec-other because this is more of a preventative measure, as far as I understand.
Keywords: sec-other, sec-want
Attachment #722394 - Flags: review?(ehsan) → review+
This may be the least reviewable patch I have ever written....
Attachment #725260 - Flags: review?(ehsan)
Attachment #722396 - Attachment is obsolete: true
Comment on attachment 725260 [details] [diff] [review]
part 2.  Null-check the pointer to the editor that the edit rules hold.

Review of attachment 725260 [details] [diff] [review]:
-----------------------------------------------------------------

This patch makes me so sad.  Thanks for writing it!  ;-)

::: editor/libeditor/html/nsHTMLEditRules.cpp
@@ +807,5 @@
>  
>    int32_t offset, rootOffset;
>    nsCOMPtr<nsIDOMNode> parent = nsEditor::GetNodeLocation(rootElem, &rootOffset);
> +  NS_ENSURE_STATE(mHTMLEditor);
> +    res = mHTMLEditor->GetStartNodeAndOffset(selection, getter_AddRefs(parent), &offset);

I assume you changed the indentation here by mistake.

@@ +6056,5 @@
>      for (i=listCount-1; i>=0; i--)
>      {
>        nsCOMPtr<nsIDOMNode> node = outArrayOfNodes[i];
>        if (!aDontTouchContent && IsInlineNode(node) 
> +           &&

Nit: please put this at the end of previous line.
Attachment #725260 - Flags: review?(ehsan) → review+
> I assume you changed the indentation here by mistake.

Yes, copy/paste brought along some spaces.

> Nit: please put this at the end of previous line.

Done.

   https://hg.mozilla.org/integration/mozilla-inbound/rev/7437279a223d
   https://hg.mozilla.org/integration/mozilla-inbound/rev/4c2a4448bd09
Flags: in-testsuite-
Whiteboard: [need to finish up patch]
Target Milestone: --- → mozilla22
Whiteboard: [adv-main22-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.