Closed
Bug 492706
Opened 16 years ago
Closed 16 years ago
Window object XOW wrapping code can leave sDoSecurityCheckInAddProperty set wrong.
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: jst, Assigned: jst)
Details
(Keywords: fixed1.9.0.12, fixed1.9.1, Whiteboard: [sg:investigate])
Attachments
(1 file, 1 obsolete file)
|
1.19 KB,
patch
|
mrbkap
:
review+
mrbkap
:
superreview+
sicking
:
approval1.9.1+
dveditz
:
approval1.9.0.12+
|
Details | Diff | Splinter Review |
There's an early return hiding inside code that sets sDoSecurityCheckInAddProperty to true, and back to false, that could leave sDoSecurityCheckInAddProperty set to the wrong value if a call to sXPConnect->GetXOWForObject() fails. It's unclear how important sDoSecurityCheckInAddProperty really is any longer, given all the wrappers that protect window objects nowadays, but still, we should fix this (or get rid of sDoSecurityCheckInAddProperty alltogether if that's the right thing to do).
Attachment #377100 -
Flags: superreview?(mrbkap)
Attachment #377100 -
Flags: review?(mrbkap)
|
Assignee | |
Comment 1•16 years ago
|
||
Attachment #377100 -
Attachment is obsolete: true
Attachment #377102 -
Flags: superreview?(mrbkap)
Attachment #377102 -
Flags: review?(mrbkap)
Attachment #377100 -
Flags: superreview?(mrbkap)
Attachment #377100 -
Flags: review?(mrbkap)
|
||
Updated•16 years ago
|
Attachment #377102 -
Flags: superreview?(mrbkap)
Attachment #377102 -
Flags: superreview+
Attachment #377102 -
Flags: review?(mrbkap)
Attachment #377102 -
Flags: review+
|
|
||
Comment 2•16 years ago
|
||
Comment on attachment 377102 [details] [diff] [review]
Found one more...
"Oops"... IIRC, the only thing sDoSecurityCheckInAddProperty protects anymore is the location property protection, which we probably don't care about. Want to file a bug about getting rid of both in one fell swoop?
|
|
Assignee | |
Comment 4•16 years ago
|
||
Need is a strong word here, but yeah, we probably should take this for 1.9.0. I think for trunk at least we'll take the fix for bug 492713 instead.
|
|
Assignee | |
Updated•16 years ago
|
Attachment #377102 -
Flags: approval1.9.1?
Attachment #377102 -
Flags: approval1.9.0.12?
Attachment #377102 -
Flags: approval1.9.1? → approval1.9.1+
|
|
Assignee | |
Comment 5•16 years ago
|
||
Fixed for 1.9.1.
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e49c05fc9122
Keywords: fixed1.9.1
|
||
Updated•16 years ago
|
Attachment #377102 -
Flags: approval1.9.0.12? → approval1.9.0.12+
|
|
||
Comment 6•16 years ago
|
||
Comment on attachment 377102 [details] [diff] [review]
Found one more...
Approved for 1.9.0.12, a=dveditz for release-drivers
|
|
||
Updated•16 years ago
|
Flags: wanted1.9.0.x? → wanted1.9.0.x+
|
|
||
Comment 8•16 years ago
|
||
(In reply to comment #4)
> I think for trunk at least we'll take the fix for bug 492713 instead.
Does that mean _this_ bug can be closed "FIXED" since it's fixed on the branches where it applies? Does the fact that bug 492713 hasn't landed yet mean you're rethinking whether you want to do it? (more likely you were just busy with 1.9.1 and vacation.)
|
|
||
Comment 9•16 years ago
|
||
Wasn't clear if this was ever an exploitable issue, but good to have it fixed.
Whiteboard: [sg:investigate]
|
|
||
Updated•16 years ago
|
Group: core-security
|
|
Assignee | |
Comment 11•16 years ago
|
||
Fixed on trunk by the fix for bug 492713.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
|
||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•