Last Comment Bug 327776 - [FIX]boxobj.setPropertyAsSupports(undefined, undefined) crashes [@ nsBoxObject::SetPropertyAsSupports]
: [FIX]boxobj.setPropertyAsSupports(undefined, undefined) crashes [@ nsBoxObjec...
: crash, fixed1.8.1, testcase, verified1.8.0.2
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: P3 critical (vote)
: mozilla1.9alpha1
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
: Hixie (not reading bugmail)
: Andrew Overholt [:overholt]
Depends on:
Blocks: 326633
  Show dependency treegraph
Reported: 2006-02-18 18:23 PST by Jesse Ruderman
Modified: 2013-04-04 13:53 PDT (History)
3 users (show)
dveditz: blocking1.8.0.2+
jruderman: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (315 bytes, text/html)
2006-02-18 18:26 PST, Jesse Ruderman
no flags Details
Proposed fix (1.44 KB, patch)
2006-02-20 17:08 PST, Boris Zbarsky [:bz] (still a bit busy)
bryner: review+
bryner: superreview+
bryner: approval‑branch‑1.8.1+
dveditz: approval1.8.0.2+
Details | Diff | Splinter Review

Description Jesse Ruderman 2006-02-18 18:23:12 PST
boxobj.setPropertyAsSupports(undefined, undefined) crashes.  Should this be fixed with a null check of some kind, or by denying content access to these objects or this function?

Marking security-sensitive because of a scary-sounding assertion in  (I haven't tested whether the testcase triggers the assertion.)
Comment 1 Jesse Ruderman 2006-02-18 18:26:41 PST
Created attachment 212362 [details]
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2006-02-20 17:08:12 PST
Created attachment 212540 [details] [diff] [review]
Proposed fix
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2006-02-20 17:09:29 PST
This is a null-pointer dereference, basically.  Not related to the assertion from comment 0.  Not sure whether this should be security sensitive; I'd guess "no".
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2006-02-20 17:17:18 PST
Comment on attachment 212540 [details] [diff] [review]
Proposed fix

I think we should take this on the 1.8.0 branch.  Simple null-check fix.
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2006-02-20 17:18:31 PST
Fixed on trunk and 1.8.1 branch.  I still think this bug should be opened up.
Comment 6 Christian :Biesinger (don't email me, ping me on IRC) 2006-02-20 17:30:35 PST
another possible fix would be to change that interface to use AString, then you can't pass null either but avoid the nullcheck (guess that wouldn't work for the branches though)
Comment 7 Daniel Veditz [:dveditz] 2006-02-21 23:32:17 PST
Comment on attachment 212540 [details] [diff] [review]
Proposed fix

approved for 1.8.0 branch, a=dveditz
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2006-02-22 18:44:54 PST
Fixed for
Comment 9 Jay Patel [:jay] 2006-03-06 15:43:34 PST
v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060306 Firefox/, no crash with testcase, just an exception in jsc:

Error: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIBoxObject.setPropertyAsSupports]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: :: init :: line 11"  data: no]
Source File:
Line: 11
Comment 10 Jesse Ruderman 2006-07-27 05:21:38 PDT
See also bug 346083, same thing for nsBoxObject::SetProperty.
Comment 11 Jesse Ruderman 2007-12-14 20:16:11 PST
Crashtest checked in.

Note You need to log in before you can comment on or make changes to this bug.