Closed
Bug 702487
Opened 13 years ago
Closed 13 years ago
crash [@ PK11PasswordPromptRunnable::RunOnTargetThread]
Categories
(Core :: Security, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: nhirata, Assigned: wesj)
Details
(Keywords: crash, qawanted, reproducible, Whiteboard: [native-crash:P4][QA^])
Crash Data
Attachments
(2 files)
91.22 KB,
text/plain
|
Details | |
4.36 KB,
patch
|
mfinkle
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-ea810b4c-1943-4a29-b46a-ca6622111107 .
=============================================================
Crashing Thread
Frame Module Signature [Expand] Source
0 libxul.so PK11PasswordPromptRunnable::RunOnTargetThread nsCharTraits.h:378
1 libxul.so PK11PasswordPrompt security/manager/ssl/src/nsNSSCallbacks.cpp:827
2 libnss3.so PK11_DoPassword pk11auth.c:535
3 libnss3.so PK11_Authenticate pk11auth.c:334
4 libxul.so nsSecretDecoderRing::Decrypt security/manager/ssl/src/nsSDR.cpp:147
5 libxul.so nsSecretDecoderRing::DecryptString security/manager/ssl/src/nsSDR.cpp:212
6 libxul.so NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:194
7 libxul.so XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:2907
8 libxul.so XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1553
9 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
10 libxul.so js::Interpret js/src/jsinterp.cpp:3948
11 libxul.so js::RunScript js/src/jsinterp.cpp:584
12 libxul.so js::Invoke js/src/jsinterp.cpp:647
13 libxul.so JS_CallFunctionValue js/src/jsapi.cpp:5144
14 libxul.so nsXPCWrappedJSClass::CallMethod js/xpconnect/src/XPCWrappedJSClass.cpp:1533
15 libxul.so nsXPCWrappedJS::CallMethod js/xpconnect/src/XPCWrappedJS.cpp:553
16 libxul.so PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:131
17 libxul.so libxul.so@0x947d0b
18 libxul.so NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:194
19 libxul.so XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:2907
20 libxul.so XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1553
21 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
22 libxul.so js::Interpret js/src/jsinterp.cpp:3948
23 libxul.so js::RunScript js/src/jsinterp.cpp:584
24 libxul.so js::Invoke js/src/jsinterp.cpp:647
25 libxul.so JS_CallFunctionValue js/src/jsapi.cpp:5144
26 libxul.so nsXPCWrappedJSClass::CallMethod js/xpconnect/src/XPCWrappedJSClass.cpp:1533
27 libxul.so nsXPCWrappedJS::CallMethod js/xpconnect/src/XPCWrappedJS.cpp:553
28 libxul.so PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:131
29 libxul.so libxul.so@0x947d0b
30 libxul.so NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:194
31 libxul.so XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:2907
32 libxul.so XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1553
33 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
34 libxul.so js::Interpret js/src/jsinterp.cpp:3948
35 libxul.so js::RunScript js/src/jsinterp.cpp:584
36 libxul.so js::Invoke js/src/jsinterp.cpp:647
37 libxul.so JS_CallFunctionValue js/src/jsapi.cpp:5144
38 libxul.so nsXPCWrappedJSClass::CallMethod js/xpconnect/src/XPCWrappedJSClass.cpp:1533
39 libxul.so nsXPCWrappedJS::CallMethod js/xpconnect/src/XPCWrappedJS.cpp:553
40 libxul.so PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:131
41 libxul.so libxul.so@0x947d0b
42 libxul.so NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/unix/xptcinvoke_arm.cpp:194
43 libxul.so XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:2907
44 libxul.so XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1553
45 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
46 libxul.so js::Interpret js/src/jsinterp.cpp:3948
47 libxul.so js::RunScript js/src/jsinterp.cpp:584
48 libxul.so js::InvokeGetterOrSetter js/src/jsinterp.cpp:647
49 libxul.so js_GetPropertyHelper js/src/jsscopeinlines.h:279
50 libxul.so js::Interpret js/src/jsinterp.cpp:3478
51 libxul.so js::RunScript js/src/jsinterp.cpp:584
52 libxul.so js::InvokeGetterOrSetter js/src/jsinterp.cpp:647
53 libxul.so js_GetPropertyHelper js/src/jsscopeinlines.h:279
54 libxul.so js::Interpret js/src/jsinterp.cpp:3478
55 libxul.so js::RunScript js/src/jsinterp.cpp:584
56 libxul.so js::InvokeGetterOrSetter js/src/jsinterp.cpp:647
57 libxul.so js_GetPropertyHelper js/src/jsscopeinlines.h:279
58 libxul.so js::Interpret js/src/jsinterp.cpp:3478
59 libxul.so js::RunScript js/src/jsinterp.cpp:584
60 libxul.so js::InvokeGetterOrSetter js/src/jsinterp.cpp:647
61 libxul.so js_GetPropertyHelper js/src/jsscopeinlines.h:279
62 libxul.so js::Interpret js/src/jsinterp.cpp:3478
63 libxul.so js::RunScript js/src/jsinterp.cpp:584
64 libxul.so js::InvokeKernel js/src/jsinterp.cpp:647
65 libxul.so js_fun_call js/src/jsinterp.h:148
66 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
67 libxul.so js::Interpret js/src/jsinterp.cpp:3948
68 libxul.so js::RunScript js/src/jsinterp.cpp:584
69 libxul.so js::InvokeKernel js/src/jsinterp.cpp:647
70 libxul.so js_fun_call js/src/jsinterp.h:148
71 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
72 libxul.so js::Interpret js/src/jsinterp.cpp:3948
73 libxul.so js::RunScript js/src/jsinterp.cpp:584
74 libxul.so js::InvokeKernel js/src/jsinterp.cpp:647
75 libxul.so js_fun_call js/src/jsinterp.h:148
76 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
77 libxul.so js::Interpret js/src/jsinterp.cpp:3948
78 libxul.so js::RunScript js/src/jsinterp.cpp:584
79 libxul.so js::InvokeKernel js/src/jsinterp.cpp:647
80 libxul.so js_fun_call js/src/jsinterp.h:148
81 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
82 libxul.so js::Interpret js/src/jsinterp.cpp:3948
83 libxul.so js::RunScript js/src/jsinterp.cpp:584
84 libxul.so js::InvokeKernel js/src/jsinterp.cpp:647
85 libxul.so js_fun_call js/src/jsinterp.h:148
86 libxul.so js::InvokeKernel js/src/jscntxtinlines.h:297
87 libxul.so js::Interpret js/src/jsinterp.cpp:3948
88 libxul.so js::RunScript js/src/jsinterp.cpp:584
89 libxul.so js::InvokeKernel js/src/jsinterp.cpp:647
90 libxul.so js::CallOrConstructBoundFunction js/src/jsinterp.h:148
91 libxul.so js::Invoke js/src/jscntxtinlines.h:297
92 libxul.so JS_CallFunctionValue js/src/jsapi.cpp:5144
93 libxul.so nsXPCWrappedJSClass::CallMethod js/xpconnect/src/XPCWrappedJSClass.cpp:1533
94 libxul.so nsXPCWrappedJS::CallMethod js/xpconnect/src/XPCWrappedJS.cpp:553
95 libxul.so PrepareAndDispatch xpcom/reflect/xptcall/src/md/unix/xptcstubs_arm.cpp:131
96 libxul.so libxul.so@0x947d0b
97 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:631
![]() |
Reporter | |
Updated•13 years ago
|
Whiteboard: [native-crash] → [native-crash], str-wanted
![]() |
Reporter | |
Updated•13 years ago
|
Whiteboard: [native-crash], str-wanted → [native-crash:P4], str-wanted
Comment 1•13 years ago
|
||
I was able to reproduce this crash on the latest Nightly build.
Steps to reproduce:
1. Open Fennec App
2. Go to Settings and set a Master Password
3. Restart app
4. Go to litmus.mozilla.org > Log in
5. Fill in email and password fields and tap on Login button
6. When Master Password popup is triggered, just tap OK button
Expected result:
OK button should be grayed out. However, even if OK button is still active, Master Password popup will be triggered again until the correct password is typed. No crash should occur after step 6.
Actual result:
After step 6, Fennec crashes. Please see the attached log.
Crash report: https://crash-stats.mozilla.com/report/index/bp-c2da091e-1f0c-48a2-b67f-fe63c2120109
--
Mozilla/5.0 (Android;Linux armv7l;rv:12.0a1)Gecko/20120108
Firefox/12.0a1 Fennec/12.0a1
Devices: Samsung Galaxy S
OS: Android 2.2
Updated•13 years ago
|
Whiteboard: [native-crash:P4], str-wanted → [native-crash:P4]
Version: 10 Branch → 12 Branch
![]() |
Reporter | |
Comment 2•13 years ago
|
||
status-firefox11:
--- → affected
status-firefox12:
--- → affected
![]() |
Reporter | |
Comment 3•13 years ago
|
||
Need regression range; still reproducible.
Keywords: regressionwindow-wanted,
reproducible
![]() |
||
Comment 4•13 years ago
|
||
According the crash report the crashing thread is not the main thread but thread number 8. Main thread (0) is just sleeping - I assume on arm thread 0 is the main thread...
But at [1] we still call runnable->RunOnTargetThread().
[1] http://hg.mozilla.org/mozilla-central/file/22f16ec4052a/security/manager/ssl/src/nsNSSCallbacks.cpp#l823
![]() |
Reporter | |
Comment 5•13 years ago
|
||
still occurs on the aurora nightly 02/07/2012
![]() |
Reporter | |
Comment 6•13 years ago
|
||
Happens since re-implementation 12/22/2012: bug 700527; there's no way to get to the dialog before that.
Keywords: regressionwindow-wanted
![]() |
Reporter | |
Comment 7•13 years ago
|
||
Comment 8•13 years ago
|
||
This issue is still reproducing on the latest Nightly (03/26). Imo, there are enough changes that an user to tap just OK button when Master Password prompt dialog is triggered. We should avoid this kind of crashes when security elements are involved such Master Password. It feels like a vulnerability
Updated•13 years ago
|
Assignee: nobody → wjohnston
blocking-fennec1.0: ? → +
Comment 9•13 years ago
|
||
It derefrences a null pointer. 4 crashes in 4 weeks. Re-noming to see if we should unblock.
blocking-fennec1.0: + → ?
Whiteboard: [native-crash:P4] → [native-crash:P4], startupcrash
Updated•13 years ago
|
Whiteboard: [native-crash:P4], startupcrash → [native-crash:P4]
Comment 10•13 years ago
|
||
This crash still occurs on the latest Nightly and it's always reproducible if steps from comment #1 are performed.
--
Firefox 14.0a1 (2012-04-04)
Device: HTC Desire Z
OS: Android 2.3.3
Updated•13 years ago
|
blocking-fennec1.0: ? → soft
Updated•13 years ago
|
Priority: -- → P2
Updated•13 years ago
|
Whiteboard: [native-crash:P4] → [native-crash:P4][QA^]
![]() |
Reporter | |
Comment 11•13 years ago
|
||
I believe you have to have a blank dialog for the master password in order to get this crash. Even though it's reproducible, the chances of someone placing in a blank master password is a lot lower. (though, the fix should just be a null check right?)
![]() |
||
Comment 12•13 years ago
|
||
(In reply to Naoki Hirata :nhirata from comment #11)
> I believe you have to have a blank dialog for the master password in order
> to get this crash. Even though it's reproducible, the chances of someone
> placing in a blank master password is a lot lower.
FTR, changing an existing master password to blank is the preferred method to removing the master password, so this is a case that should work.
Updated•13 years ago
|
blocking-fennec1.0: soft → -
Updated•13 years ago
|
tracking-firefox15:
--- → ?
tracking-firefox16:
--- → ?
Comment 13•13 years ago
|
||
This is a low volume crasher, so I'm curious why we think this needs to track for 15/16 release. Is this affecting testing or our users significantly? It's not clear to me that comment 1 is a common user scenario.
Comment 14•13 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #13)
> This is a low volume crasher, so I'm curious why we think this needs to
> track for 15/16 release. Is this affecting testing or our users
> significantly? It's not clear to me that comment 1 is a common user scenario.
Yes, it is. Actually, when master password popup is triggered, just tap the OK button to reproduce this crash.
Comment 15•13 years ago
|
||
It just means that this feature is not used much. But it is ridiculously easy to hit this crash when using this feature.
Updated•13 years ago
|
Updated•13 years ago
|
Assignee: wjohnston → nobody
Comment 16•13 years ago
|
||
Wes - any updates here? We'd like to get a fix into all branches asap.
Assignee: nobody → wjohnston
Assignee | ||
Comment 17•13 years ago
|
||
Sorry! Hadn't seen this. This should fix this, plus some other potential problems. When users cancel out of an dialog, we should hit onCancel:
http://mxr.mozilla.org/mozilla-aurora/source/mobile/android/base/PromptService.java#320
which should mean we don't return anything, but we don't change the entered values in that case anyway.
If they hit OK, with this we will return the values entered regardless of what they are. Before we only returned them if they were truthy.
CheckState.value is independent of that I think? i.e. If it says "Remember this choice" callers likely want to remember that you hit cancel.
Attachment #643700 -
Flags: review?(mark.finkle)
Updated•13 years ago
|
Attachment #643700 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 18•13 years ago
|
||
I'm nervous about pushing this forward until its had a little bake time and QA has looked at it. I want to hold off on nom'ing.
https://hg.mozilla.org/integration/mozilla-inbound/rev/5191b82f2976
Comment 19•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Assignee | ||
Comment 20•13 years ago
|
||
Comment on attachment 643700 [details] [diff] [review]
Patch
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 694455.
User impact if declined: Crashes in this case because we're talking to Gecko code that hates the null response. Most cases web pages will just receive erroneous information.
Testing completed (on m-c, etc.): Been on mc a week.
Risk to taking this patch (and alternatives if risky): Mobile prompt service only. Involves correcting the prompt service behavior and since that interacts with lots of Gecko and web pages, its slightly risky IMO.
String or UUID changes made by this patch: None.
Attachment #643700 -
Flags: approval-mozilla-beta?
Attachment #643700 -
Flags: approval-mozilla-aurora?
Comment 21•13 years ago
|
||
Comment on attachment 643700 [details] [diff] [review]
Patch
[Triage Comment]
Since this is slightly risky, let's start on Aurora 16 and then evaluate for Beta 15 later this week.
Attachment #643700 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Updated•13 years ago
|
status-firefox16:
--- → affected
Assignee | ||
Comment 22•13 years ago
|
||
Updated•13 years ago
|
Comment 23•13 years ago
|
||
Adding qawanted - can someone run the steps in comment 1 on Aurora so we can determine if this is verified fixed before consideration for Beta?
Keywords: qawanted
Comment 24•13 years ago
|
||
Cannot access Firefox Settings due to Bug 776909. I will verify this bug when the patch for the mentioned crash will land.
Comment 25•13 years ago
|
||
(In reply to Lukas Blakk [:lsblakk] from comment #23)
> Adding qawanted - can someone run the steps in comment 1 on Aurora so we can
> determine if this is verified fixed before consideration for Beta?
I cannot reproduce this crash on the latest Aurora build.
--
Firefox 16.0a2 (2012-07-25)
Device: Acer Iconia TAB A500
OS: Android 3.2
status-firefox11:
affected → ---
status-firefox12:
affected → ---
Updated•13 years ago
|
Comment 26•13 years ago
|
||
Comment on attachment 643700 [details] [diff] [review]
Patch
[Triage Comment]
Given positive feedback on Aurora, uplifting to Beta 15.
Attachment #643700 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 27•13 years ago
|
||
Updated•13 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•