6.49 KB, patch
|Details | Diff | Splinter Review|
125 bytes, text/html
816 bytes, patch
|Details | Diff | Splinter Review|
So, the testcase doesn't work for me on my debug build. I get "Setting a property that only has a getter" because the navigator property is supposed to be read only! This patch should work anyway.
Assignee: dveditz → mrbkap
Status: NEW → ASSIGNED
Attachment #226691 - Flags: review?(dveditz)
Dan, is that testcase really for this bug?
The first one isn't going to work loaded from bugzilla... I didn't seem to need the refresh to reproduce, but if you can't save this attachment and the exp_loader one locally and load it from there.
Attachment #226717 - Attachment is obsolete: true
So loading the mail exploit itself seems to just crash, locally I do need to load the loader page first to get the exploit to run correctly. Here's an attempt to do just that on BMO.
> So loading the mail exploit itself seems to just crash, locally I do need to > load the loader page first to get the exploit to run correctly. Here's an > attempt to do just that on BMO. Doesn't seem to work (launch calc.exe) on a known broken version. Still get the crash, but for full effect download attachments 226717 and 226722 locally.
So Firefox 1.0 and mozilla 1.7 are not vulnerable because bug 246224 ("Mozilla crashes if a chrome app uses Live Connect") was fixed on those branches by removing the only call to ProxyFindClass http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/modules/oji/src/ProxyJNI.cpp&rev=184.108.40.206&mark=322,325,328#315
Comment on attachment 226691 [details] [diff] [review] Fix That should do nicely, r=dveditz You mention this bug didn't happen to you on the trunk because navigator can't be overridden. I'm not seeing that, I crash. This should land on the trunk regardless: either it fixes the crash, or *will* fix it if the readonly attribute stops being enforced again.
Attachment #226691 - Flags: review?(dveditz) → review+
Attachment #226691 - Flags: superreview?(jst)
Comment on attachment 226691 [details] [diff] [review] Fix sr=jst
Attachment #226691 - Flags: superreview?(jst) → superreview+
I can't believe I missed this before: the liveconnect code checks for JSVAL_IS_OBJECT and if that tests true, then it passes the jsval (as a JSObject *) to JS_GetPrivate. Unfortunately, null is an object, and JS_GetPrivate is not null-safe.
Comment on attachment 226808 [details] [diff] [review] Null-defense patch r/sr=dveditz
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 226808 [details] [diff] [review] Null-defense patch approved for 1.8.0 branch, a=dveditz for drivers
Attachment #226808 - Flags: approval220.127.116.11? → approval18.104.22.168+
Comment on attachment 226691 [details] [diff] [review] Fix oops, forgot to mark the main patch. Of course we want this on the 1.8.0 branch also, a=dveditz for drivers
Attachment #226691 - Flags: approval22.214.171.124? → approval126.96.36.199+
Fix checked into the 1.8.0 branch.
Comment on attachment 226691 [details] [diff] [review] Fix a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #226691 - Flags: approval1.8.1? → approval1.8.1+
Fix checked into the 1.8 branch.
v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:188.8.131.52) Gecko/20060626 Firefox/184.108.40.206, no crash with testcases and no calc.exe execution (was able to execute with 6/20 build).
Whiteboard: [sg:critical] requires Java (on by default) → [sg:critical] requires Java (on by default) 1.7/aviary not vulnerable
You need to log in before you can comment on or make changes to this bug.