Closed
Bug 1138669
(CVE-2015-2742)
Opened 10 years ago
Closed 10 years ago
Some mac crash reports include private information
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox38 | --- | wontfix |
firefox38.0.5 | --- | wontfix |
firefox39 | --- | fixed |
firefox-esr31 | --- | wontfix |
firefox-esr38 | --- | wontfix |
b2g-v2.0 | --- | unaffected |
b2g-v2.0M | --- | unaffected |
b2g-v2.1 | --- | unaffected |
b2g-v2.1S | --- | unaffected |
b2g-v2.2 | --- | unaffected |
b2g-master | --- | unaffected |
People
(Reporter: handyman, Assigned: handyman)
Details
(Keywords: sec-low, Whiteboard: [adv-main39+])
Attachments
(1 file)
1.73 KB,
patch
|
smichaud
:
review+
|
Details | Diff | Splinter Review |
I'm working on bug 1124408 and the crash reports I'm viewing contain private information. The bug is about a crash when using some password fields (like the password manager). In the crash report, actual keys that the user pressed when they got the crash are plainly visible. For example:
https://crash-stats.mozilla.com/report/index/5f846d1b-cff1-455d-86dc-6dd4e2150301
From the AppNotes section, I can see that this user has a 'k' in their password. This information is valuable to the bug but also clearly private. I don't know the policy here but I suspect this info should be removed from the report.
The message is logged from the Cocoa widget object.
Comment 1•10 years ago
|
||
I'm inclined to think this isn't really a problem. We don't know the account name, and we only know one letter of the password.
Dan, what do you think?
Flags: needinfo?(dveditz)
Comment 3•10 years ago
|
||
What code is adding this to the crash report? Why do we need to record this information in the crash report?
I can certainly think of ways that sites could try to abuse this, given enough time. I think that if we need this data, we should move it into a private metadata field and not the public app notes.
Flags: needinfo?(smichaud)
Comment 4•10 years ago
|
||
The code is here:
https://hg.mozilla.org/mozilla-central/annotate/993eb76a8bd6/widget/cocoa/nsChildView.mm#l5375
We're just recording the native key event that triggers a crash.
I really do think this is harmless. But it may no longer be needed -- especially as we seem to be closing in on fixes for this class of crashes.
What do you think, David?
Flags: needinfo?(smichaud) → needinfo?(davidp99)
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Steven Michaud from comment #4)
>
> I really do think this is harmless. But it may no longer be needed --
> especially as we seem to be closing in on fixes for this class of crashes.
I'd say 'closing in' is accurate, but the bugs I've been handling related to this are still popping up. I don't know if its still got 'top crasher' status but I have been using the incidental reports of "I'm still seeing this crash" to determine if the core issue is resolved. I'd really like to keep the crash. That said, we can afford to drop theEvent from the log message. Its utility is marginal. Alternatively, we could just write the parts of the event that don't include key info.
This patch removes the entire event from the message, in case we decide to go that way.
Flags: needinfo?(davidp99)
Comment 6•10 years ago
|
||
If you still need it, please put it in a separate annotation instead of the app notes.
Comment 7•10 years ago
|
||
(In reply to Steven Michaud from comment #1)
> I'm inclined to think this isn't really a problem. We don't know the
> account name, and we only know one letter of the password.
+1
(In reply to Benjamin Smedberg [:bsmedberg] from comment #6)
> If you still need it, please put it in a separate annotation instead of the
> app notes.
At least for bug 893973, we have a lot of reports but we still don't fine the hint from the inputting character. So, I think that removing input character from the log is okay but I'm not sure for the other developers.
Comment 8•10 years ago
|
||
Comment on attachment 8573539 [details] [diff] [review]
Remove theEvent from nsChildView crash log message
So none of the developers who've worked on this problem think the native key event is critically important to resolving this bug. Then let's get rid of it, at least temporarily.
Attachment #8573539 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → davidp99
Keywords: checkin-needed
Comment 9•10 years ago
|
||
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment 11•10 years ago
|
||
Safe to call this wontfix for the ESRs?
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.0M:
--- → unaffected
status-b2g-v2.1:
--- → unaffected
status-b2g-v2.1S:
--- → unaffected
status-b2g-v2.2:
--- → unaffected
status-b2g-master:
--- → unaffected
status-firefox38:
--- → wontfix
status-firefox38.0.5:
--- → wontfix
status-firefox-esr31:
--- → ?
status-firefox-esr38:
--- → ?
Flags: needinfo?(davidp99)
Assignee | ||
Comment 12•10 years ago
|
||
I'm not clear on how these calls are made. Steven?
Flags: needinfo?(davidp99) → needinfo?(smichaud)
Comment 13•10 years ago
|
||
> Safe to call this wontfix for the ESRs?
Yes. The code that was patched only runs in non-release builds.
Flags: needinfo?(smichaud)
Updated•10 years ago
|
Updated•10 years ago
|
Whiteboard: [adv-main39+]
Updated•10 years ago
|
Alias: CVE-2015-2742
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•