324746 - XPathResult object can crash brower when calling iterateNext() or snapshotItem()

Status: RESOLVED FIXED

(Core :: XSLT, defect)

Description

[warp56]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

The XPathResult object returned from a document.evaluate() query can cause the browser to crash when trying to call the XPathResult's iterateNext() or snapshotItem() methods.  This happens when a query that should return a number value (such as "count(//*)") is evaluated and the XPathResult type argument is specifically set to one of the ITERATOR or SNAPSHOT types. Clearly it is an error of the script writer who set the incorrect XPath query or incorrect XPathResult type, but the application should just throw an exception rather than crashing.

Crashes on both Firefox 1.5 and Firefox 1.6a1 on Windows XP.

Reproducible: Always

Steps to Reproduce:
1. Run any of the following Javascript lines in a webpage:
document.evaluate("count(/*)",document,null,XPathResult.UNORDERED_NODE_SNAPSHOT_TYPE,null).snapshotItem(0);
document.evaluate("count(/*)",document,null,XPathResult.UNORDERED_NODE_ITERATOR_TYPE,null).iterateNext();
document.evaluate("count(/*)",document,null,XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,null).snapshotItem(0);
document.evaluate("count(/*)",document,null,XPathResult.ORDERED_NODE_ITERATOR_TYPE,null).iterateNext();
Actual Results:  
Browser crashed.

Expected Results:  
An exception is thrown which can either be caught, or gets logged to the console.

Module crash in firefox.exe.

Comment 1

[warp56]

Attachment: Testcase for XPathResult crash

This is a simple testcase that should show the results of this bug.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: stack

Comment 3

[:Gavin Sharp [email: [e-mail]]]

The stack above was obtained from a Firefox trunk build from earlier today, on Windows.

Comment 4

[Peter Van der Beken [:peterv]]

Attachment: v1

We need to throw on impossible conversions (which I think is limited to "not a nodeset to an iterator, snapshot or node").

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 209710 [details] [diff] [review]
v1

sr=jst

Comment 6

[Peter Van der Beken [:peterv]]

Comment on attachment 209710 [details] [diff] [review]
v1

Simple crash fix, low risk: just throw on certain conditions instead of crashing later on.

Comment 7

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

Comment on attachment 209710 [details] [diff] [review]
v1

Peterv is module owner so landing request over to him

Comment 8

[Peter Van der Beken [:peterv]]

Comment on attachment 209710 [details] [diff] [review]
v1

Crash fix.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 209710 [details] [diff] [review]
v1

approved for 1.8.0 branch, a=dveditz

Comment 10

[Dave Liebreich [:davel]]

Marking [rft-dl] (ready for testing in Firefox 1.5.0.2 release candidates)

Comment 11

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060302 Firefox/1.5.0.1, no crash with testcase.