Closed Bug 394107 Opened 15 years ago Closed 14 years ago

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


(Core :: Widget: Cocoa, defect)

Not set





(Reporter: zillaster, Assigned: jaas)



(Whiteboard: [sg:low] local exploit)


(1 file, 4 obsolete files)

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: Gecko/20070725 Firefox/

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:

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.

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.


Reproducible: Always

Steps to Reproduce:
I can supply a test application to demonstrate this security vulnerability. Please email me at
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.

FYI, this bug also affects Camino 1.5.1.
Bug 394106 is the Camino bug on this.
Component: Security → Event Handling
Product: Firefox → Core
QA Contact: firefox → events
Target Milestone: --- → mozilla1.9 M9
Version: unspecified → Trunk
Assignee: nobody → smichaud
Flags: blocking1.9+
Putting this in cocoa widgets, seems most appropriate for now.
Assignee: smichaud → joshmoz
Component: Event Handling → Widget: Cocoa
QA Contact: events → cocoa
Assignee: joshmoz → smichaud
Ever confirmed: true
Duplicate of this bug: 394106
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?]
Attached patch trunk fix v1.0 (obsolete) — Splinter Review
This should fix the problem on trunk, probably ports easily to the 1.8 branch.
Assignee: smichaud → joshmoz
Attachment #278715 - Flags: review?(smichaud)
Attached patch fix v1.1 (obsolete) — Splinter Review
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)
Attachment #278722 - Flags: review?(stuart.morgan) → review+
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?
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)?
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.
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

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
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).
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+
(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.
Attached patch fix v1.2 (obsolete) — Splinter Review
Uses an nsIWidget API instead of ifdefs.
Attachment #278722 - Attachment is obsolete: true
Attachment #282353 - Flags: superreview?(roc)
Attachment #278722 - Flags: superreview?(dbaron)
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).
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?
Attached patch fix v1.3 (obsolete) — Splinter Review
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)
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+
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?
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?


> 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.
Attached patch fix v1.4Splinter Review
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+
landed on trunk
Closed: 14 years ago
Resolution: --- → FIXED
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-
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.
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.
[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.
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-
Depends on: 424350
Duplicate of this bug: 422539
Group: security
You need to log in before you can comment on or make changes to this bug.