71773 - Crash when reading past the end of a CSSRuleList

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Hixie (not reading bugmail)]

STEPS TO REPRODUCE
   1. <style type="text/css"></style>
   2. document.styleSheets[0].cssRules[0];

ACTUAL RESULTS
   KABOOM!

See http://www.damowmow.com/mozilla/crash/8.html

Comment 1

[Fabian Guisset]

confirmed on mozilla0.8 windows95...

Comment 2

[Hixie (not reading bugmail)]

http://www.damowmow.com/mozilla/crash/9.html shows this problem too, but, I 
think, because another bug is causing the array to be smaller than expected.

In other words: This problem is causing other problems to be more serious! :-)

Comment 3

[Hixie (not reading bugmail)]

Attachment: Patch: moves the NS_RELEASE(), and adds code to deal with the NS_ERROR_ILLEGAL_VALUE case

Comment 4

[Hixie (not reading bugmail)]

The patch moves the NS_RELEASE() to inside the |if (result == NS_OK)| block,
since |rule| will only be allocated if |result| is |NS_OK|. This fixes the 
crash. The patch also adds an |else if| for the NS_ERROR_ILLEGAL_VALUE case,
so that we follow the DOM spec, which says to return null in that case.

Test case:
   http://www.damowmow.com/mozilla/crash/8.html

Looking for r=, sr=, and someone to check it in...

Comment 5

[Hixie (not reading bugmail)]

Attachment: Patch: Switches to using nsCOMPtr instead of NS_RELEASE(), changes from QueryInterface to CallQueryInterface, and checks for |rule| instead of NS_OK.

Comment 6

[Hixie (not reading bugmail)]

New patch, this time doing all the Right Things per jst. :-)

Once again looking for r=, sr=, and someone to check it in...

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

r=glazman ; tested on linux

Comment 8

[Johnny Stenback (:jst)]

Thank you Hixie for the fix (and Daniel for the review), I'll land this once the
tree opens, sr=jst

Comment 9

[Blake Ross]

Fix checked in.

Comment 10

[Johnny Stenback (:jst)]

*** Bug 73274 has been marked as a duplicate of this bug. ***