Closed
Bug 422825
Opened 16 years ago
Closed 16 years ago
uninitialized memory-read in XPCWrapper::AddProperty
Categories
(Core :: XPConnect, defect, P1)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla1.9beta5
People
(Reporter: crowderbt, Assigned: crowderbt)
References
Details
(Keywords: regression, testcase)
Attachments
(1 file)
932 bytes,
patch
|
crowderbt
:
review+
crowderbt
:
superreview+
ted
:
approval1.9+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #420585 +++ The if (!isXOW) sequence in XPCWrapper::AddProperty() seems to allow a situation where |v| is used in OBJ_DEFINE_PROPERTY uninitialized. I am hitting this case while using firebug 1.2 with a recent nightly build. This patch may be wrong. Shouldn't we init as *vp? mrbkap suggested JSVAL_VOID on IRC, and I have used that to continue my work, but I'm not sure what is more correct. I'm also not sure how to architect a testcase that hits this issue. Filing as security-sensitive, for now, though I'm very sure this is a trunk-only (and recent, at that) bug.
Assignee | ||
Comment 1•16 years ago
|
||
Woops. Lost my summary in a minefield crash.
Summary: Defining setters for properties of the global object doesn't work → uninitialized memory-read in XPCWrapper::AddProperty
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → crowder
Flags: blocking1.9?
Priority: -- → P1
Target Milestone: --- → mozilla1.9beta5
Updated•16 years ago
|
Comment 2•16 years ago
|
||
Ah, I forgot we had a *vp coming in there. r+sr=me on using it as an initial value.
Assignee | ||
Comment 4•16 years ago
|
||
This has r/sr from mrbkap and needs a+
Attachment #309415 -
Flags: superreview+
Attachment #309415 -
Flags: review+
Attachment #309415 -
Flags: approval1.9?
Comment 5•16 years ago
|
||
Comment on attachment 309415 [details] [diff] [review] init v a=me for crash fix
Attachment #309415 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 6•16 years ago
|
||
Thanks, Ted. If I'm not around when the tree reopens, I'm hoping someone will land. Killing sec-sensitive flag on this UMR bug, since it is trunk-only, recent, and soon-to-be-patched.
Group: security
Keywords: checkin-needed
Assignee | ||
Comment 7•16 years ago
|
||
XPCWrapper.cpp:1.25
Assignee | ||
Comment 8•16 years ago
|
||
This deserves a testcase, but I confess I'm not sure how to concoct one that excercises this line of code; I know only that firebug hits it.
Flags: in-testsuite?
Assignee | ||
Comment 9•16 years ago
|
||
Clearing blocking, since this has a+ and has landed.
Flags: blocking1.9?
You need to log in
before you can comment on or make changes to this bug.
Description
•