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

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Doug Hill]

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 [e-mail]
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.
[e-mail]

FYI, this bug also affects Camino 1.5.1.

Comment 1

[Samuel Sidler (old account; do not CC)]

Bug 394106 is the Camino bug on this.

Comment 2

[Josh Aas]

Putting this in cocoa widgets, seems most appropriate for now.

Comment 4

[Josh Aas]

Even though this is in cocoa widgets, this affects Gecko 1.8 as well, just so nobody misses that.

Comment 5

[Josh Aas]

Attachment: trunk fix v1.0

This should fix the problem on trunk, probably ports easily to the 1.8 branch.

Comment 6

[Josh Aas]

Attachment: fix v1.1

Turns out we can only get global status for enabled status, we have to keep track of our own calls.

Comment 7

[Daniel Veditz [:dveditz]]

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?

Comment 8

[Josh Aas]

We can't use actual native Cocoa controls so NSSecureTextField is not an option.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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)?

Comment 10

[Josh Aas]

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

[Stuart Morgan]

It's the latter, so they are already safe in current Camino.

Comment 12

[Steven Michaud [:smichaud] (Retired)]

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.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

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).

Comment 14

[Josh Aas]

Right, a branch patch would have to weak-link against those symbols.

Comment 15

[Doug Hill]

(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.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I would much prefer a new nsIWidget API for this instead of sticking #ifdefs into nsTextControlFrame.

Comment 18

[Josh Aas]

Attachment: fix v1.2

Uses an nsIWidget API instead of ifdefs.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  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).

Comment 20

[Josh Aas]

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?

Comment 21

[Josh Aas]

Attachment: fix v1.3

Updated patch, assumes lose focus or destroy before focus on another widget. Waiting to hear back from roc before requesting review.

Comment 22

[Josh Aas]

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

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 282363 [details] [diff] [review]
fix v1.3

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

I think you just need "if (mInSecureKeyboardInputMode)" here.

Comment 24

[Josh Aas]

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?

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> 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.

Comment 26

[Josh Aas]

Attachment: fix v1.4

Addresses all comments. This does fix the problem using Doug's test app which he emailed to me.

Comment 27

[Josh Aas]

landed on trunk

Comment 28

[Josh Aas]

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.

Comment 29

[Josh Aas]

I'm not sure we should take this on the branch. The risk associated doesn't seem to be worth the benefit. dveditz?

Comment 30

[Samuel Sidler (old account; do not CC)]

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.

Comment 31

[Josh Aas]

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.

Comment 32

[Doug Hill]

[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.

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 34

[Doug Hill]

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.

Comment 35

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 36

[Daniel Veditz [:dveditz]]

Not realistically going to get a fix for this on the 1.8 branch. Should discuss unhiding this bug in that case.