159484 - javascript urls can steal password data

Status: VERIFIED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Description

[Daniel Veditz [:dveditz]]

Password manager makes the same kind of host comparing mistake as in bug 152725,
thus javascript urls can steal password data.

Comment 1

[Daniel Veditz [:dveditz]]

Attachment: testcase

This isn't a full exploit, but if you have password manager stores your
bugzilla password then clicking on the link will load that data into a form.

Comment 2

[Daniel Veditz [:dveditz]]

Attachment: scarier testcase -- will reveal your bugzilla password to passers by

Comment 3

[Daniel Veditz [:dveditz]]

This is definitely a stop-ship.

Talon doesn't support Password Manager, does it?

Comment 4

[Stephen P. Morse]

Attachment: check to see if scheme supports hostnames

Comment 5

[Daniel Veditz [:dveditz]]

Should we worry about SINGSIGN_RememberSignonData? Is there a way someone could
*save* a password for another host? If so how bad would that be?

Comment 6

[Stephen P. Morse]

I had thought about that but couldn't see any problem.  What would an attacker
gain by having you log on as him -- he could simply do that himself.  Also,
before a password is saved, the user must respond affirmatively to a pop-up
alert, so this can't happen without his knowledge.

Comment 7

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 93053 [details] [diff] [review]
check to see if scheme supports hostnames

Hmm, this patch looks oddly familiar. r=mstoltz.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Attachment: Fix for 0.9.4 branch

Comment 9

[chris hofmann]

Comment on attachment 93053 [details] [diff] [review]
check to see if scheme supports hostnames

a=chofmann for branch and trunk

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 93053 [details] [diff] [review]
check to see if scheme supports hostnames

I can't think of a specific exploit against RememberSignonData, but it's clear
that the end result is known bad data getting into the password file if the
user clicks the wrong button (or another exploit suppresses the dialog?). I
don't know what an attacker could do with it if he succeeded, either, but bogus
data in such a sensitive spot sets off all my paranoia bells.

sr=dveditz if you file a bug on the RememberSignonData spot, or a bug to
convert wallet uses of nsIIOService to nsIURI. This patch is a stopgap anyway,
you're verifying that the nsIURI returns a host, but the host you actually do
use is gotten from a different method and may not in fact match.

Comment 11

[Stephen P. Morse]

OK, I agree.  Better safe than sorry.  So I'll add the RememberSignonData patch
here as well.

Comment 12

[Stephen P. Morse]

Attachment: check in two places

Comment 13

[Stephen P. Morse]

Comment on attachment 93158 [details] [diff] [review]
check in two places

bringing reviews forward:
r=mstoltz
sr=dveditz
a=chofmann

Comment 14

[Stephen P. Morse]

Landed on trunk.

Comment 15

[Jaime Rodriguez, Jr.]

bsharma: can you pls verify this one on the trunk tomorrow? thanks!

Comment 16

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch #2 for 0.9.4 branch - apply this AFTER applying the first one

Comment 17

[bsharma]

Verified on 2002-07-30-trunk on Win 2000.

Steps:
1. Saved the bugzilla password in the Password Manager.
2. Ran the attached test case.
3. An exception is thrown in the JS console.
4. Ran the form test case.
5. An exception is thrown in the JS console.

Comment 18

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
drivers ' approval. pls check this in asap, then replace the "mozilla1.0.1+"
with "fixed1.0.1". thanks!

Comment 19

[Stephen P. Morse]

Patch checked in on branch.

Comment 20

[chris hofmann]

a=chofmann for 1.0.1