1158452 - Out of bounds stack read in mozilla::dom::Element::ParseAttribute

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Atte Kettunen]

Attachment: repro-file.html

Tested on:

OS: Ubuntu 14.04

Firefox: ASAN build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64-asan/1429905257/

ASAN-trace:

==32503==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff966358a8 at pc 0x7fd82a45d59e bp 0x7fff966353a0 sp 0x7fff96635398
READ of size 4 at 0x7fff966358a8 thread T0 (Web Content)
    #0 0x7fd82a45d59d in IsEmpty /home/attekett/firefox/objdir-ff-asan/dom/base/../../dist/include/nsTSubstring.h:242:5
    #1 0x7fd82a45d59d in mozilla::dom::Element::ParseAttribute(int, nsIAtom*, nsAString_internal const&, nsAttrValue&) /home/attekett/firefox/dom/base/Element.cpp:2373:0
    #2 0x7fd82a834270 in nsStyledElementNotElementCSSInlineStyle::ParseAttribute(int, nsIAtom*, nsAString_internal const&, nsAttrValue&) /home/attekett/firefox/dom/base/nsStyledElement.cpp:43:10
    #3 0x7fd82cb5a590 in nsGenericHTMLElement::ParseAttribute(int, nsIAtom*, nsAString_internal const&, nsAttrValue&) /home/attekett/firefox/dom/html/nsGenericHTMLElement.cpp:1031:10
    #4 0x7fd82c9c6e47 in mozilla::dom::HTMLDivElement::ParseAttribute(int, nsIAtom*, nsAString_internal const&, nsAttrValue&) /home/attekett/firefox/dom/html/HTMLDivElement.cpp:59:10
    #5 0x7fd82a45a6dc in mozilla::dom::Element::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) /home/attekett/firefox/dom/base/Element.cpp:2195:8
    #6 0x7fd82cbab69a in nsGenericHTMLElement::SetAttr(int, nsIAtom*, nsIAtom*, nsAString_internal const&, bool) /home/attekett/firefox/dom/html/nsGenericHTMLElement.cpp:905:17
    #7 0x7fd82c9db27f in SetAttr /home/attekett/firefox/objdir-ff-asan/dom/html/../../dist/include/mozilla/dom/Element.h:446:12
    #8 0x7fd82c9db27f in SetId /home/attekett/firefox/objdir-ff-asan/dom/html/../../dist/include/mozilla/dom/Element.h:594:0
    #9 0x7fd82c9db27f in SetId /home/attekett/firefox/dom/html/nsGenericHTMLElement.h:337:0
    #10 0x7fd82c9db27f in non-virtual thunk to nsGenericHTMLElement::SetId(nsAString_internal const&) /home/attekett/firefox/dom/html/nsGenericHTMLElement.h:417:0
    #11 0x7fd82df68ec9 in nsHTMLEditor::RelativeFontChangeOnTextNode(int, nsIDOMCharacterData*, int, int) /home/attekett/firefox/editor/libeditor/nsHTMLEditorStyle.cpp:1542:3
.
.
.
Address 0x7fff966358a8 is located in stack of thread T0 (Web Content) at offset 264 in frame
    #0 0x7fd82df68b9f in nsHTMLEditor::RelativeFontChangeOnTextNode(int, nsIDOMCharacterData*, int, int) /home/attekett/firefox/editor/libeditor/nsHTMLEditorStyle.cpp:1521:0

  This frame has 10 object(s):
    [32, 40) 'node.i13'
    [64, 80) 'rv.i14'
    [96, 104) 'node.i'
    [128, 144) 'rv.i'
    [160, 168) 'textNode'
    [192, 200) 'tmp'
    [224, 232) 'node'
    [256, 260) 'textLen' <== Memory access at offset 264 overflows this variable
    [272, 280) 'sibling' <== Memory access at offset 264 underflows this variable
    [304, 312) ''
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)

Comment 1

[Boris Zbarsky [:bzbarsky]]

nsHTMLEditorStyle.cpp:1542 for that revision looks to me like:

  aTextNode->GetLength(&textLen);

How are we landing in SetId?

Well, in nsHTMLEditor::RelativeFontChangeOnTextNode we have this:

  nsIDOMCharacterData *aTextNode

but my debugger says:

(gdb) x/wa (*(void**)aTextNode)
0x10a4beb58 <_ZTVN7mozilla3dom14HTMLDivElementE+2296>:  0x438a060

which might kinda sorta explain what happens with that GetLength() call, I guess: some vtable confusion.

The caller here looks like this:

1508            res = RelativeFontChangeOnTextNode(aDir == FontSize::incr ? +1 : -1,
1509                static_cast<nsIDOMCharacterData*>(startNode->AsDOMNode()),
1510                0, range->EndOffset());

but startNode.mRawPtr is an HTMLDivElement... and this is inside an IsTextNode(endNode) if statement, too.  Looks like a copy/paste error in bug 1154701 part 9.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Pass in the right node when messing with font sizes in editor

Comment 3

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef79ced353a6

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ef79ced353a6