Last Comment Bug 327776 - [FIX]boxobj.setPropertyAsSupports(undefined, undefined) crashes [@ nsBoxObject::SetPropertyAsSupports]
: [FIX]boxobj.setPropertyAsSupports(undefined, undefined) crashes [@ nsBoxObjec...
Status: VERIFIED FIXED
[rft-dl]
: 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]
: Hixie (not reading bugmail)
Mentors:
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: ---


Attachments
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]
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 http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/xul/base/src/nsBoxObject.cpp&rev=1.53#405.  (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]
testcase
Comment 2 Boris Zbarsky [:bz] 2006-02-20 17:08:12 PST
Created attachment 212540 [details] [diff] [review]
Proposed fix
Comment 3 Boris Zbarsky [:bz] 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] 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] 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] 2006-02-22 18:44:54 PST
Fixed for 1.8.0.2.
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:1.8.0.2) Gecko/20060306 Firefox/1.5.0.2, 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 :: https://bugzilla.mozilla.org/attachment.cgi?id=212362 :: init :: line 11"  data: no]
Source File: https://bugzilla.mozilla.org/attachment.cgi?id=212362
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.