Closed Bug 382527 Opened 15 years ago Closed 14 years ago

execCommand inserthtml triggers out-of-bounds memory read

Categories

(Core :: DOM: Editor, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [sg:low?])

Attachments

(2 files)

Loading the testcase triggers a bunch of assertions, including one that indicates an out-of-bounds memory read.


###!!! ASSERTION: invalid action: 'aAction == eNext || aAction == ePrevious', file editor/libeditor/base/nsEditor.cpp, line 4926

###!!! ASSERTION: invalid direction: 'aDirection == eNext || aDirection == ePrevious', file editor/libeditor/base/nsEditor.cpp, line 4893

###!!! ASSERTION: index exceeds allowable range: 'i <= mLength', file ../../dist/include/string/nsTString.h, line 133

###!!! ASSERTION: bad arg, numCharsToDelete.  Not enough characters in node: 'count>=aNumCharsToDelete', file editor/libeditor/base/DeleteTextTxn.cpp, line 73

###!!! ASSERTION: could not get text to delete.: 'NS_SUCCEEDED(result)', file editor/libeditor/base/DeleteTextTxn.cpp, line 90

###!!! ASSERTION: transaction did not execute properly: '(NS_SUCCEEDED(result))', file editor/libeditor/base/nsEditor.cpp, line 734

###!!! ASSERTION: we reached a null node ancestor !: 'node', file editor/libeditor/html/nsHTMLCSSUtils.cpp, line 1422
Attached file testcase
Whiteboard: [sg:low?]
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Assignee: nobody → Olli.Pettay
Attached patch possible patchSplinter Review
Contains similar changes as what was done in bug 382778.
The fix for this bug is adding the |if (aAction != eNone)|.
Removed also that #ifdef DEBUG_akkana, which has wrong checks in the
assertion.
Attachment #277938 - Flags: review?(daniel)
Status: NEW → ASSIGNED
Attachment #277938 - Flags: review?(daniel) → review?(neil)
Comment on attachment 277938 [details] [diff] [review]
possible patch

Because Jesse has thoughtfully created us a selection with two collapsed ranges in it we'll notify listeners and perform an empty transaction. Is this going to break anyone? I guess it would have before, so...

Note: this review dependent on nobody deciding that a selection consisting of collapsed ranges is collapsed.
Attachment #277938 - Flags: review?(neil) → review+
Attachment #277938 - Flags: superreview?(peterv)
Attachment #277938 - Flags: superreview?(peterv) → superreview+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Branch shows only one assertion, and it's not the scary one, so I'm making this bug report public.
Group: security
Crashtest checked in.
Flags: in-testsuite+
Flags: wanted1.8.1.x-
You need to log in before you can comment on or make changes to this bug.