pkcs11.addmodule needs to protect against bug 162020-style attacks

RESOLVED FIXED

Status

()

Core
Security: PSM
RESOLVED FIXED
13 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: kaie)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86
Windows XP
csectype-spoof, fixed1.8.0.2, fixed1.8.1, sec-moderate
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.0.2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tcn-dl][qa:verified-tb-1802])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
Splitting from bug 326628.

Kai Engert (:kaie) wrote in bug 326628 comment 7:

> > What could a malicious page do if it could get you to add a module through
> > addmodule?  Could it take over your computer?

> Besides adding a module, an attacker would have do to one more thing: Install 
> a malicious executable shared library / dll on the user's machine, which 
> contains the entry points expected by NSS.

> If such a library gets installed and added to the user's profile using
> addmodule(), NSS will call functions on the library. That code will run with
> the user's permissions on the operating system level.

I doubt it's hard for a web site to get a so/dll of its choice in a semi-predictable location on a user's machine.  Convincing a user to alt+click somewhere would probably be sufficient, or a bug162020-style attack could be used with the save-as dialog.

Once that's accomplished, a bug162020-style attack could easily be used against the pkcs11.addmodule dialog, and the attacker would be able to run arbitrary code with the user's permissions.

I think the best solution is to make pkcs11.addmodule use the extension-installation dialog, since that dialog includes the appropriate warnings and protection from accidental clicks (including ones with focus changes).  I don't know if just using that dialog would work, though.
(Reporter)

Updated

13 years ago
Blocks: 326628, 326633
Flags: blocking1.8.0.3?
Flags: blocking1.8.0.2?
Whiteboard: [sg:critical?]
To be critical it assumes some other critical bug that installs local code on the user's machine. Otherwise you've got the delete which might mess people up, a DoS that the user could reinstall to fix. No fix, don't see how this could make 1.8.0.2
Flags: blocking1.8.0.2? → blocking1.8.0.2-
Whiteboard: [sg:critical?] → [sg:moderate]
(Reporter)

Comment 2

13 years ago
Being able to place a dll in a semi-predictable location (not a location of the attacker's choice, but predictable) would not be considered an sg:critical bug.  I'm not convinced it would be considered a security hole at all.  And whether you consider it a security hole or not, it's possible now in multiple ways (see my first paragraph in comment 0).

So I'm pretty sure this bug is sg:critical.
Whiteboard: [sg:moderate] → [sg:critical]
Changing text isn't going to happen on 1.8.0.x, the safest/quickest change is to switch from confirm() to confirmEx() in confirm_user, to make

http://lxr.mozilla.org/mozilla/source/security/manager/ssl/src/nsCrypto.cpp#2529

look something like

http://lxr.mozilla.org/mozilla/source/caps/src/nsScriptSecurityManager.cpp#2404

except without, for the 1.8.0 branch, the custom button titles. That would help the "clarity" bug 326628
severity->moderate. Places like Secunia rated our 162020 "Moderate", and Microsoft rated their similar bug "Moderate". Our own "critical" rating on 162020 has confused people doing cross-product security comparisons such as Washington Post reporter Brian Krebs recently.

With a quick fix such as outlined in comment 3 we might be able to get this into 1.8.0.2 if it can land on trunk for verification nearly immediately.
Flags: blocking1.8.0.2- → blocking1.8.0.2?
Whiteboard: [sg:critical] → [sg:moderate]
(Reporter)

Comment 5

13 years ago
I think critical was the correct rating for bug 162020 (and similar bugs in other browsers).

Comment 6

13 years ago
Kai-  Any status on a fix?  We will leave it "?" until a fix is available.
(Assignee)

Comment 7

13 years ago
Created attachment 213899 [details] [diff] [review]
Patch v1

Dan, is that what you are proposing?
Attachment #213899 - Flags: superreview?(dveditz)
(Assignee)

Comment 8

13 years ago
Created attachment 213903 [details] [diff] [review]
Patch v2

Actually, I like this patch better.
This keeps our current OK/Cancel button labels, so any existing documentation will still be in sync with UI.

The only thing new will be the delay, before it's possible to press OK.
Attachment #213899 - Attachment is obsolete: true
Attachment #213903 - Flags: superreview?(dveditz)
Attachment #213899 - Flags: superreview?(dveditz)
(Assignee)

Comment 9

13 years ago
Bob and Wan-Teh, FYI: this bug proposes to add a few seconds security delay before the user can confirm installation/removal of pkcs#11 modules.
Comment on attachment 213903 [details] [diff] [review]
Patch v2

sr=dveditz
approved for 1.8.0 branch, a=dveditz for drivers.

Please land immediately!
Attachment #213903 - Flags: superreview?(dveditz)
Attachment #213903 - Flags: superreview+
Attachment #213903 - Flags: approval1.8.0.2+
(Assignee)

Comment 11

13 years ago
Comment on attachment 213903 [details] [diff] [review]
Patch v2

Bob, are you ok with this change?
Attachment #213903 - Flags: review?(rrelyea)
(Assignee)

Comment 12

13 years ago
I verified that adding and deleting modules still works correctly with the patch applied.

Comment 13

13 years ago
Comment on attachment 213903 [details] [diff] [review]
Patch v2

r+ = relyea
Attachment #213903 - Flags: review?(rrelyea) → review+
Flags: blocking1.8.0.3?
Flags: blocking1.8.0.2?
Flags: blocking1.8.0.2+
(Assignee)

Updated

13 years ago
Attachment #213903 - Flags: approval-branch-1.8.1+
(Assignee)

Updated

13 years ago
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8.0.2, fixed1.8.1
Resolution: --- → FIXED
Please provide testcase and testing guidance on how to verify this fix.
Whiteboard: [sg:moderate] → [sg:moderate][tcn-dl]
(Assignee)

Comment 15

13 years ago
You can't really verify unless you install some additional crypto software package on your system. Without, you are not able to install and remove.

However, you can "nearly" verify it.

Go to
  http://kuix.de/misc/test326628/

It will suggest you to install a module on your system. You most likely do not have that file on your disk, so even if you confirm with OK, the operation will return a failure.

However, without the patch, you'll see that all buttons are immediately clickable.

With the patch, there is a 2 seconds delay until you are able to clock OK. This is the fix.

The same thing happens with removing modules. You can't verify that, unless you succeed installing. But we are using the same shared code for install/remove.

Comment 16

12 years ago
verified fixed windows xp/tbird/20060308 with js enabled sending an html message containing http://kuix.de/misc/test326628/
Whiteboard: [sg:moderate][tcn-dl] → [sg:moderate][tcn-dl][qa:verified-tb-1802]
Group: security
(Reporter)

Updated

5 years ago
Keywords: csec-spoof, sec-moderate
Whiteboard: [sg:moderate][tcn-dl][qa:verified-tb-1802] → [tcn-dl][qa:verified-tb-1802]
You need to log in before you can comment on or make changes to this bug.