Last Comment Bug 159484 - javascript urls can steal password data
: javascript urls can steal password data
Status: VERIFIED FIXED
[adt1 rtm] [ETA 07/30]
:
Product: SeaMonkey
Classification: Client Software
Component: Passwords & Permissions (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla1.0.1
Assigned To: Stephen P. Morse
: bsharma
Mentors:
Depends on:
Blocks: 143047
  Show dependency treegraph
 
Reported: 2002-07-25 19:58 PDT by Daniel Veditz [:dveditz]
Modified: 2004-11-22 17:25 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
testcase (870 bytes, text/html)
2002-07-25 20:02 PDT, Daniel Veditz [:dveditz]
no flags Details
scarier testcase -- will reveal your bugzilla password to passers by (787 bytes, text/html)
2002-07-25 21:18 PDT, Daniel Veditz [:dveditz]
no flags Details
check to see if scheme supports hostnames (1.21 KB, patch)
2002-07-27 22:27 PDT, Stephen P. Morse
security-bugs: review+
dveditz: superreview+
chofmann: approval+
Details | Diff | Splinter Review
Fix for 0.9.4 branch (1.21 KB, patch)
2002-07-28 19:17 PDT, Mitchell Stoltz (not reading bugmail)
no flags Details | Diff | Splinter Review
check in two places (2.28 KB, patch)
2002-07-29 09:26 PDT, Stephen P. Morse
morse: review+
morse: superreview+
morse: approval+
Details | Diff | Splinter Review
Patch #2 for 0.9.4 branch - apply this AFTER applying the first one (1.25 KB, patch)
2002-07-30 11:43 PDT, Mitchell Stoltz (not reading bugmail)
no flags Details | Diff | Splinter Review

Description Daniel Veditz [:dveditz] 2002-07-25 19:58:41 PDT
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] 2002-07-25 20:02:08 PDT
Created attachment 92843 [details]
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] 2002-07-25 21:18:18 PDT
Created attachment 92854 [details]
scarier testcase -- will reveal your bugzilla password to passers by
Comment 3 Daniel Veditz [:dveditz] 2002-07-26 10:28:18 PDT
This is definitely a stop-ship.

Talon doesn't support Password Manager, does it?
Comment 4 Stephen P. Morse 2002-07-27 22:27:59 PDT
Created attachment 93053 [details] [diff] [review]
check to see if scheme supports hostnames
Comment 5 Daniel Veditz [:dveditz] 2002-07-27 23:36:11 PDT
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 2002-07-27 23:51:39 PDT
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) 2002-07-28 17:02:13 PDT
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) 2002-07-28 19:17:50 PDT
Created attachment 93119 [details] [diff] [review]
Fix for 0.9.4 branch
Comment 9 chris hofmann 2002-07-28 20:32:46 PDT
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] 2002-07-29 09:05:46 PDT
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 2002-07-29 09:25:35 PDT
OK, I agree.  Better safe than sorry.  So I'll add the RememberSignonData patch
here as well.
Comment 12 Stephen P. Morse 2002-07-29 09:26:44 PDT
Created attachment 93158 [details] [diff] [review]
check in two places
Comment 13 Stephen P. Morse 2002-07-29 09:28:19 PDT
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 2002-07-29 13:18:40 PDT
Landed on trunk.
Comment 15 Jaime Rodriguez, Jr. 2002-07-29 14:24:12 PDT
bsharma: can you pls verify this one on the trunk tomorrow? thanks!
Comment 16 Mitchell Stoltz (not reading bugmail) 2002-07-30 11:43:31 PDT
Created attachment 93309 [details] [diff] [review]
Patch #2 for 0.9.4 branch - apply this AFTER applying the first one
Comment 17 bsharma 2002-07-30 11:59:37 PDT
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. 2002-07-30 12:08:32 PDT
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 2002-07-30 12:20:04 PDT
Patch checked in on branch.
Comment 20 chris hofmann 2002-07-30 12:21:27 PDT
a=chofmann for 1.0.1

Note You need to log in before you can comment on or make changes to this bug.