Closed Bug 702487 Opened 13 years ago Closed 13 years ago

crash [@ PK11PasswordPromptRunnable::RunOnTargetThread]

Categories

(Core :: Security, defect, P2)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
mozilla17
Tracking Status
firefox14 - affected
firefox15 + fixed
firefox16 --- verified
blocking-fennec1.0 --- -

People

(Reporter: nhirata, Assigned: wesj)

Details

(Keywords: crash, qawanted, reproducible, Whiteboard: [native-crash:P4][QA^])

Crash Data

Attachments

(2 files)

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
Whiteboard: [native-crash] → [native-crash], str-wanted
Whiteboard: [native-crash], str-wanted → [native-crash:P4], str-wanted
Attached file log
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
Whiteboard: [native-crash:P4], str-wanted → [native-crash:P4]
Version: 10 Branch → 12 Branch
Need regression range; still reproducible.
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
still occurs on the aurora nightly 02/07/2012
Happens since re-implementation 12/22/2012: bug 700527; there's no way to get to the dialog before that.
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
blocking-fennec1.0: --- → ?
Version: 12 Branch → Trunk
Assignee: nobody → wjohnston
blocking-fennec1.0: ? → +
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
Whiteboard: [native-crash:P4], startupcrash → [native-crash:P4]
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
blocking-fennec1.0: ? → soft
Priority: -- → P2
Whiteboard: [native-crash:P4] → [native-crash:P4][QA^]
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?)
(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.
blocking-fennec1.0: soft → -
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.
(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.
It just means that this feature is not used much. But it is ridiculously easy to hit this crash when using this feature.
Assignee: wjohnston → nobody
Wes - any updates here? We'd like to get a fix into all branches asap.
Assignee: nobody → wjohnston
Attached patch PatchSplinter Review
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)
Attachment #643700 - Flags: review?(mark.finkle) → review+
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
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
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 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+
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
Cannot access Firefox Settings due to Bug 776909. I will verify this bug when the patch for the mentioned crash will land.
(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
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+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: