The default bug view has changed. See this FAQ.

CGEvent taps on Macintosh OS X can steal HTML form passwords.

RESOLVED FIXED in mozilla1.9beta1

Status

()

Core
Widget: Cocoa
--
critical
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: Doug Hill, Assigned: Josh Aas)

Tracking

Trunk
mozilla1.9beta1
PowerPC
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
blocking1.8.1.12 -
wanted1.8.1.x +
blocking1.8.0.14 -
wanted1.8.0.x +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:low] local exploit)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/522.11.1 (KHTML, like Gecko) Version/3.0.3 Safari/522.12.1
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6

On Macintosh OS X 10.3 and later, any application can create an Event Tap to capture events as they are sent to the HID system (input devices). In this case, all events sent by all input devices to every process will be intercepted by the tap. Apple realized this could be a major security problem and implemented a way for processes to disallow events to password fields to be intercepted. See:
http://developer.apple.com/technotes/tn2007/tn2150.html

The gist of the tech note is for developers to call EnableSecureEventInput when the user types into a password text field (or other sensitive information). Cocoa developers get this behavior for free when using a Cocoa NSSecureTextField.

Problem:
Firefox doesn't seem to be using either of the above schemes to protect HTML password entry form text input fields. I was able to write an event recording using CGEvent Taps and could record password text typed into a password HTML form text entry field.
This means Firefox is vulnerable to key loggers that are very easy to write. I was able to create a proof of concept in less than a few hours.

THIS HAS TO BE REGARDED AS A TOP-PRIORITY SECURITY BREACH!

Reproducible: Always

Steps to Reproduce:
I can supply a test application to demonstrate this security vulnerability. Please email me at bugzilla@elasmobranch.com
Actual Results:  
Text typed into a password entry field in an HTML form is captured by a CG Event tap.

Expected Results:  
Key down/up events are not sent to the event tap.

Please contact me about getting my test application if would like to reproduce this bug.
bugzilla@elasmobranch.com

FYI, this bug also affects Camino 1.5.1.
Bug 394106 is the Camino bug on this.
(Assignee)

Updated

10 years ago
Component: Security → Event Handling
Product: Firefox → Core
QA Contact: firefox → events
Target Milestone: --- → mozilla1.9 M9
Version: unspecified → Trunk
(Assignee)

Updated

10 years ago
Assignee: nobody → smichaud
Flags: blocking1.9+
(Assignee)

Comment 2

10 years ago
Putting this in cocoa widgets, seems most appropriate for now.
Assignee: smichaud → joshmoz
Component: Event Handling → Widget: Cocoa
QA Contact: events → cocoa
(Assignee)

Updated

10 years ago
Assignee: joshmoz → smichaud
Status: UNCONFIRMED → NEW
Ever confirmed: true
Duplicate of this bug: 394106
(Assignee)

Comment 4

10 years ago
Even though this is in cocoa widgets, this affects Gecko 1.8 as well, just so nobody misses that.
Flags: blocking1.8.1.7?
Flags: blocking1.8.0.14?
Whiteboard: [sg:moderate?]
(Assignee)

Comment 5

10 years ago
Created attachment 278715 [details] [diff] [review]
trunk fix v1.0

This should fix the problem on trunk, probably ports easily to the 1.8 branch.
Assignee: smichaud → joshmoz
Status: NEW → ASSIGNED
Attachment #278715 - Flags: review?(smichaud)
(Assignee)

Comment 6

10 years ago
Created attachment 278722 [details] [diff] [review]
fix v1.1

Turns out we can only get global status for enabled status, we have to keep track of our own calls.
Attachment #278715 - Attachment is obsolete: true
Attachment #278722 - Flags: review?(stuart.morgan)
Attachment #278715 - Flags: review?(smichaud)

Updated

10 years ago
Attachment #278722 - Flags: review?(stuart.morgan) → review+
(Assignee)

Updated

10 years ago
Attachment #278722 - Flags: superreview?(dbaron)
Would it make sense to use this only for the Firefox 1.8 branch, and use NSSecureTextField where we're using cocoa widgets (trunk, Camino trunk and branch)? Or is that painful given DOM scripts access could change the type of an <input> at any time?
(Assignee)

Comment 8

10 years ago
We can't use actual native Cocoa controls so NSSecureTextField is not an option.
Is only content vulnerable, or do we need to worry about chrome, too, e.g., the httpAuth login (which is a real sheet in Camino, and a XUL one in Firefox) or the  Master Password stuff in Firefox prefs, or the PSM/NSS "Software Security Device" password stuff (again native in Camino, and XUL in Firefox)?
(Assignee)

Comment 10

10 years ago
If the sheet in Camino is a XUL sheet, then this should fix it. If the sheet in Camino is not a XUL sheet, then you're using NSSecureTextFields and you aren't vulnerable.

Comment 11

10 years ago
It's the latter, so they are already safe in current Camino.
This patch is needed, and a good idea.  But there are some mitigating
circumstances.

For example (according to Apple's doc on CGEventTapCreate()):

> Event taps receive key up and key down events if one of the
> following conditions is true:
>
> * The current process is running as the root user.
> * Access for assistive devices is enabled. In Mac OS X v10.4, you
>   can enable this feature using System Preferences, Universal Access
>   panel, Keyboard view.

(From comment #0)

> Actual Results:
> Text typed into a password entry field in an HTML form is captured
> by a CG Event tap.

Doug, was your event tap running as root, or did you enable access for
assistive devices?

Whether you did or not, though, it looks like GetKeys() opens the barn
door even to non-root apps (if we don't use EnableSecureEventInput()).

I don't know whether you need to be root to use
IOHIDDeviceInterface->open.
Something to be aware of when we fix this on the branch:

According to Apple's TN2150, EnableSecureEventInput() is only
supported on OS X 10.3 and above.  Which means that we may need to
leave this problem unfixed on OS X 10.2.8 (which is technically still
supported on the branch).
(Assignee)

Comment 14

10 years ago
Right, a branch patch would have to weak-link against those symbols.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.7?
Flags: blocking1.8.1.7+
(Reporter)

Comment 15

10 years ago
(In reply to comment #12)
> Doug, was your event tap running as root, or did you enable access for
> assistive devices?

I have "Enable Access for Assistive Devices" enabled.
Duplicate of this bug: 394380
I would much prefer a new nsIWidget API for this instead of sticking #ifdefs into nsTextControlFrame.
(Assignee)

Comment 18

10 years ago
Created attachment 282353 [details] [diff] [review]
fix v1.2

Uses an nsIWidget API instead of ifdefs.
Attachment #278722 - Attachment is obsolete: true
Attachment #282353 - Flags: superreview?(roc)
Attachment #278722 - Flags: superreview?(dbaron)

Updated

10 years ago
Target Milestone: mozilla1.9 M9 → ---
+  while (sEnableSecureOSXInputCalls > 0) {
+    ::DisableSecureEventInput();
+    sEnableSecureOSXInputCalls--;
+  }

Why do this? If we miss a call to EndSecureKeyboardInput, we'll have event taps disabled for an unacceptably long period of time anyway.

I suggest just documenting that Begin/EndSecureKeyboardInput cannot be nested, and add a debug-only boolean and assertions that fire if someone tries to call BeginSecureKeyboardInput while we're already in that mode. Then we should be able to catch bugs from it not being re-enabled.

I think you should add something to nsTextControlFrame to record whether you've called BeginSecureKeyboardInput on behalf of the frame. Then we should do an emergency EndSecureKeyboardInput if that flag is set during Destroy(). I'm pretty sure you can destroy a frame without an intervening Blur() (e.g. if someone sets display:none on a focused element).
(Assignee)

Comment 20

10 years ago
If we're not going to allow nested calls to enable/disable then we need to enforce that in nsTextInputListener, but can we assume that only one has focus at a time?

Put another way, can we assume that after a field enables secure input mode that the field will lose focus or be destroyed before we call focus on another field?

Put yet another way, do we need to only track whether we're in secure input mode through a local variable or do we need a static also to make sure that only one field enables secure input mode?
(Assignee)

Comment 21

10 years ago
Created attachment 282363 [details] [diff] [review]
fix v1.3

Updated patch, assumes lose focus or destroy before focus on another widget. Waiting to hear back from roc before requesting review.
Attachment #282353 - Attachment is obsolete: true
Attachment #282353 - Flags: superreview?(roc)
(Assignee)

Comment 22

10 years ago
Also my patch doesn't do the EndSecureKeyboardInput on destroy yet.
Comment on attachment 282363 [details] [diff] [review]
fix v1.3

+  if (mFrame->IsPasswordTextControl() && mInSecureKeyboardInputMode) {

I think you just need "if (mInSecureKeyboardInputMode)" here.
Attachment #282363 - Flags: superreview+
(Assignee)

Comment 24

10 years ago
Why is that? Why would we want to do this for non-password text fields? Also, I'm confused about why you gave sr+ for this because of comments 20 and 22. Can you respond to comment #20?
(Assignee)

Updated

10 years ago
Target Milestone: --- → mozilla1.9 M9
> Why would we want to do this for non-password text fields?

We won't, because mInSecureKeyboardInputMode will be false. Checking IsPasswordTextControl is just superfluous.

> but can we assume that only one has focus at a time?

Yes.

> Also my patch doesn't do the EndSecureKeyboardInput on destroy yet.

I was trigger-happy, but that's the only issue that still needs to be addressed IMHO.
(Assignee)

Comment 26

10 years ago
Created attachment 282469 [details] [diff] [review]
fix v1.4

Addresses all comments. This does fix the problem using Doug's test app which he emailed to me.
Attachment #282363 - Attachment is obsolete: true
Attachment #282469 - Flags: superreview?(roc)
Attachment #282469 - Flags: superreview?(roc)
Attachment #282469 - Flags: superreview+
Attachment #282469 - Flags: approval1.9+
(Assignee)

Comment 27

10 years ago
landed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 28

10 years ago
I don't think this needs to block the currently wrapping up security release. It isn't that critical, and I'd like to have more time for feedback on the trunk fix.
Flags: blocking1.8.1.8+ → blocking1.8.1.9+
Flags: blocking1.8.0.14? → blocking1.8.0.14-
(Assignee)

Comment 29

9 years ago
I'm not sure we should take this on the branch. The risk associated doesn't seem to be worth the benefit. dveditz?
Isn't worth the benefit how? The branch will live on for at least 6 months after the release of Firefox 3 and will, likely, have a couple/few million users in that time.

How much risk is there? I don't see a statement on that above.
(Assignee)

Comment 31

9 years ago
The security risk (not much) just doesn't seem worth a patch like this to layout, especially one that depends on focus behavior which I don't have a lot of faith in. The patch is going to be even more complicated and risky on branch because the API doesn't exist on 10.2. In order to exploit this you already have to have the permission to run native code, in which case you could do plenty of other nasty things even with a patch like this.
(Reporter)

Comment 32

9 years ago
[Message from bug submitter]

I just wanted to throw in $.02 here FWIW.

I would have thought this would be considered a top-priority security breach, since one can create a password logger in a couple of hours that can steal passwords silently on ALL Macintosh Mozilla apps. I'm guessing the reason this bug isn't considered a high priority is because the user's machine needs to have GUI scripting turned on (Access for Assistive Devices preference), or the current user have root access, for the keystroke logger to work. While it's harder to get root access for a malicious application, the Assistive Device access preference has a good chance of being enabled. This preference is also used for GUI scripting so it is often enabled for many users to take advantage of this feature. Also, a malicious application can turn on this preference via scripting, although it involves convincing the user to enter an administrative password (not always too hard).

My point is that the permissions to make a keystroke logger/password stealer are often enabled on many users' systems, or can be turned on by a malicious application, thus leaving all Mozilla Macintosh applications vulnerable.

I hope this adds some perspective on this bug.
The reason this isn't considered high priority is that it requires someone to have malicious code executing locally on your machine in a normal process context. And if someone has achieved that, there are lots of things they can do that are as bad or worse as password sniffing:
-- If the process is running as root, you're obviously doomed.
-- If the process is running as you, then you're doomed because, among other things, the malicious code can for example ptrace the running Firefox process.
-- If the process can convince you to enter an administrative password, you're doomed.
And so on. So blocking this event tap stuff only provides meaningful protection in a very small range of contrived scenarios.
(Reporter)

Comment 34

9 years ago
Here's a not so contrived scenario:

- User doesn't have to be an Administrator.

- User has "Access for Assistive Devices" System Preference enabled
(This is often the case for an average user since this preference is used for GUI scripting, which is used by many applications. Many Adobe apps, e.g.)

- Someone gets the user to download and run a trojan.
(Not that tough to do in many cases)

- Trojan Application records passwords for every Mozilla application.

FYI, trojan application wouldn't need to get the user to enter an administrator password. Also, trojan app wouldn't need to use ptrace or any tricks to get any information from Mozilla apps, and would be resistant to ASLR; it uses supported System SPI's.

In this scenario, the trojan application has many options for getting the user's passwords, including replacing all the user's applications with trojaned versions.
Using ptrace is another option, and is no more of a "trick" than using a CGEvent tap. ASLR is irrelevant.
Whiteboard: [sg:moderate?] → [sg:low] local exploit
Not realistically going to get a fix for this on the 1.8 branch. Should discuss unhiding this bug in that case.
Flags: blocking1.8.1.12+ → blocking1.8.1.12-

Updated

9 years ago
Depends on: 424350

Updated

9 years ago
Duplicate of this bug: 422539

Updated

9 years ago
Group: security
Depends on: 556873
Duplicate of this bug: 288275
You need to log in before you can comment on or make changes to this bug.