102809 - ABR in nsPlainTextSerializer::AppendText; indexing -1 on array

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[John Morrison]

I was looking for something else, but happened to come up with this instead.

This is part of the checkin for bug 97687.

I'm getting an Array Bounds Read from Purify when running Mozilla. 
Specifically, I can trigger it by making a selection of part of the text in
the URLbar and cutting the selection to the clipboard. Purify info is
below. When I break in the debugger, |mTagStackIndex| is '0x00000000', so
|mTagStack[mTagStackIndex-1]| is Ooops! (Not sure why it isn't flagging it
as an ABW, since it is storing to that address, but...)


[E] ABR: Array bounds read in nsPlainTextSerializer::AppendText(nsIDOMText 
*,int,int,nsAString&) {3 occurrences}
    Reading 4 bytes from 0x09be7144 (4 bytes at 0x09be7144 illegal)
    Address 0x09be7144 is 4 bytes before the beginning of a 2000 byte block at 
0x09be7148
    Address 0x09be7144 points to a C++ new block in heap 0x02c70000
    Thread ID: 0x474
    Error location
    nsPlainTextSerializer::AppendText(nsIDOMText *,int,int,nsAString&) 
[nsPlainTextSerializer.cpp:230]
                                      nsAWritableString& aStr)
    {
      if ((mTagStack[mTagStackIndex-1] == eHTMLTag_noscript) &&
 =>       !(mFlags & nsIDocumentEncoder::OutputNoScriptContent)) {
        return NS_OK;
      }
    
    nsDocumentEncoder::SerializeNodeStart(nsIDOMNode *,int,int,nsAString&) 
[nsDocumentEncoder.cpp:315]
    nsDocumentEncoder::SerializeRangeToString(nsIDOMRange *,nsAString&) 
[nsDocumentEncoder.cpp:861]
    nsDocumentEncoder::EncodeToString(nsAString&) [nsDocumentEncoder.cpp:914]
    nsCopySupport::HTMLCopy(nsISelection *,nsIDocument *,short) 
[nsCopySupport.cpp:119]
    PresShell::DoCopy(void) [nsPresShell.cpp:4324]
    nsPlaintextEditor::Copy(void) [nsPlaintextEditor.cpp:1419]
    nsPlaintextEditor::Cut(void) [nsPlaintextEditor.cpp:1390]
    nsCutCommand::DoCommand(nsAString const&,nsISupports *) 
[nsEditorCommands.cpp:131]
    nsControllerCommandManager::DoCommand(nsAString const&,nsISupports *) 
[nsControllerCommandManager.cpp:199]
Allocation location
    new(UINT)      [msvcrt.DLL]
    nsPlainTextSerializer::nsPlainTextSerializer(void) 
[nsPlainTextSerializer.cpp:126]
          mStartedOutput = PR_FALSE;
        
          // initialize the tag stack to zero:
     =>   mTagStack = new nsHTMLTag[TagStackSize];
          mTagStackIndex = 0;
        
          // initialize the OL stack, where numbers for ordered lists are kept:
    NS_NewPlainTextSerializer(nsIContentSerializer * *) 
[nsPlainTextSerializer.cpp:87]
    CreatePlainTextSerializer [nsContentModule.cpp:308]
    nsComponentManager::CreateInstance(char const*,nsISupports *,nsID 
const&,void * *) [nsComponentManager.cpp:3146]
    nsCreateInstanceByContractID::()(nsID const&,void * *)const 
[nsComponentManager.cpp:164]
    nsDocumentEncoder::EncodeToString(nsAString&) [nsDocumentEncoder.cpp:888]
    nsCopySupport::HTMLCopy(nsISelection *,nsIDocument *,short) 
[nsCopySupport.cpp:119]
    PresShell::DoCopy(void) [nsPresShell.cpp:4324]
    nsPlaintextEditor::Copy(void) [nsPlaintextEditor.cpp:1419]

Comment 1

[John Morrison]

Attachment: purify output (more readable form).

Comment 2

[John Morrison]

need to resolve this before bug 97687 can (safely) go on the branch. (Maybe 
this is bogus, but I don't think so, since I can see the bad indexing in the
debugger without purify).

Comment 3

[John Morrison]

(Um, duh. '==' is not '=', so ABR it is. Pretty unlikely that this would give a 
false positive match to the enum, so most harmless, I suppose).

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: Fix

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 51811 [details] [diff] [review]
Fix

r=sicking

Comment 6

[John Morrison]

(Man, I meant to assign this peterv ... thanks for the fix.)

Comment 7

[Chris Waterson]

Comment on attachment 51811 [details] [diff] [review]
Fix

sr=waterson

Comment 8

[Peter Van der Beken [:peterv]]

Checked in. Thanks to all involved.

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Let's get this on the branch. 

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Um, reopening to make this onto PDT radar. 

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Argh, I should read my bugmail first. This went in on the branch with bug 97687,
closing again. Sorry for the noise...

Comment 12

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Bugzilla needs natural language parser to act on my comments, darnit!

Comment 13

[John Morrison]

verified no more ABR in purify.