Last Comment Bug 252486 - Add option to disable saving form data on https websites
: Add option to disable saving form data on https websites
Status: RESOLVED FIXED
[good first bug]
: privacy
Product: Toolkit
Classification: Components
Component: Form Manager (show other bugs)
: Trunk
: All All
: -- enhancement with 10 votes (vote)
: mozilla1.9.3a5
Assigned To: Eddy Ferreira
:
Mentors:
: 296398 505954 542986 544369 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-07-21 11:09 PDT by ken
Modified: 2011-06-23 15:14 PDT (History)
20 users (show)
bugs: blocking‑aviary1.0PR-
bugs: blocking‑aviary1.0-
dolske: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch to add requested functionality (8.91 KB, patch)
2010-03-16 17:39 PDT, Eddy Ferreira
dolske: review-
Details | Diff | Review
Patch v2 (14.86 KB, patch)
2010-04-02 21:47 PDT, Eddy Ferreira
dolske: review+
Details | Diff | Review
Patch v.3 (16.81 KB, patch)
2010-04-06 18:42 PDT, Justin Dolske [:Dolske]
no flags Details | Diff | Review

Description ken 2004-07-21 11:10:00 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040719 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040719 Firefox/0.9.1+

There should be an option to disable Saved Form for secured website because
everytime I'm buying something online, my credit card number is saved and I have
to clear all the saved form.

Reproducible: Always
Steps to Reproduce:
1. go to a https site
2. enter credit card #

Actual Results:  
credit card numbers is saved

Expected Results:  
credit card numbers shouldn't be saved
Comment 1 Bogdan Stroe 2004-07-21 13:51:11 PDT
I noticed this too, and I agree with this request.
I didn't find other bugs requesting the same thing, so marking NEW.
Also, asking for blocking 1.0 and 1.0RC1 releases.
Comment 2 Bogdan Stroe 2004-07-21 17:18:04 PDT
ladude626-cow: only the drivers and main contributors of Mozilla project can
grant a blocking request, so I will change the flags back to "?"
Comment 3 Robert Rothenberg 2004-12-31 04:48:46 PST
I just noticed this bug too.  It's a pretty serious issue, so I've disabled
saving form fields for now.

On a more advanced stage, the ability to specify which sites to save form fields
for should be added. Perhaps with it asking for the first time one fills out a
form on a page if one wants to save it (if an "ask me" option is checked)... I
think IE already does this.  There should also be a button or menu option to
manually do the same (so annoying question dialogs can be disabled).

Comment 4 Jesse Ruderman 2005-08-01 16:51:48 PDT
See also bug 190700, which seeks to change the default.
Comment 5 Robert Jessop 2005-08-04 11:24:23 PDT
Could we not come up with a good heuristic to make it do the right thing without
you having to mess with the preferences? 

How about checking that the field contains something other than digits and
whitespace. This would stop it saving credit card numbers and bank details
without breaking the feature altogether. Comments?
Comment 6 Jeremy Vanderburg 2005-08-04 12:56:52 PDT
The only issue I see with doing that is you would also prevent it from saving 
zip codes, and telephone numbers, which aren't quite as sensitive as credit 
card numbers.  Other than that, it would probably work.
Comment 7 Robert Rothenberg 2005-08-04 16:15:09 PDT
(In reply to comment #5)
> Could we not come up with a good heuristic to make it do the right thing without
> you having to mess with the preferences? 
> 
> How about checking that the field contains something other than digits and
> whitespace. This would stop it saving credit card numbers and bank details
> without breaking the feature altogether. Comments?

Not good. What if somebody wants to save a credit card number or bank PIN (as
dumb as the idea is)?  Comment #6 noted problems with thinks like zip codes and
telephone numbers.

Also: my bank asks for random digits from my PIN number. I can't have the form
saved because the field changes every time.

I should be simple to add the option to preferences.
Comment 8 Robert Rothenberg 2005-08-04 16:28:46 PDT
Perhaps another option is to specify something in the META elements of a web
site recommending that certain form fields not be saved?  Mozilla can be updated
to recognize this and not save those form fields, or at least notify the user
that some fields will not be saved.

Comment 9 morpheustdk 2006-01-10 11:16:23 PST
I first noticed this problem with FF1.04 and it is still present with 1.5. There is no need for any potental solution to get too complicated, simply disabling form history on secure sites would be enough although a solution with a potential black/whitelist of fields wonderful but possibly over the top. For credit card details most forms seem to use fields that start cc_ or credit* etc. so these could be blacklisted by default along with other common names for ensitive data in the full bells and whistles solution..


As a workaround the offending entries can be manually deleted using the Form history manager extension from http://www.radebatz.net/mano/mozilla/extensions/formhistory
Comment 10 Mike Beltzner [:beltzner, not reading bugmail] 2006-01-19 01:00:17 PST
Does this always happen? I could swear (and am pretty sure I just proved to myself) that there are many sites that ask me for my credit card number but never remember it in the form history ...
Comment 11 :Gavin Sharp [email: gavin@gavinsharp.com] 2006-01-19 01:06:52 PST
(In reply to comment #10)
> Does this always happen? I could swear (and am pretty sure I just proved to
> myself) that there are many sites that ask me for my credit card number but
> never remember it in the form history ...

Most sites that remember credit cards numbers explicitly disable "remembering" via autocomplete="off", I don't think this is a common case.
Comment 12 Mike Beltzner [:beltzner, not reading bugmail] 2006-01-19 01:14:11 PST
cc'ing mconnor and jruderman for good measure here, but I think that if there's a way for websites to keep our form manager from remembering sensitive information like credit card numbers, and especially if that seems to be a well established practise on the net, then this would cause more headaches than it would solve (as pointed out in comment 6).
Comment 13 Jesse Ruderman 2006-01-19 01:52:24 PST
Comment 5 and comment 6 are more about bug 188285, "Form autocomplete should not store credit card numbers", than about this bug.
Comment 14 Mike Beltzner [:beltzner, not reading bugmail] 2006-01-19 02:00:49 PST
Jesse, dveditz, I trust your instincts on this. If you think it's a strong enough issue to create a secondary pref in the Content > Saved Forms options for "don't save information in forms on secure web sites" then re-open this bug, plz. 

(my recommendation for the default for such a pref, needless to say, would be off)
Comment 15 Robert Rothenberg 2006-01-19 10:44:18 PST
Please reopen this.  Without this option, form manager is unusable.
Comment 16 VanillaMozilla 2007-02-02 11:35:07 PST
It shouldn't even be optional.  You don't just leave it to users to find some obscure setting.  Does Firefox remember sensitive information or doesn't it?  The answer should be NO.  Just disable autocompletion for https pages.  I don't need autocompletion in Bugzilla if it means a security problem.

I'd leave this WONTFIX but reopen bug 190700.
Comment 17 Daniel Veditz [:dveditz] 2007-04-24 08:07:40 PDT
There are enough people feeling strongly about this (see the dupe-tree for bug 190700) that we should reconsider.
Comment 18 Jesse Ruderman 2007-09-10 12:20:15 PDT
*** Bug 296398 has been marked as a duplicate of this bug. ***
Comment 19 Jesse Ruderman 2007-09-10 12:25:06 PDT
See also bug 385741, "Want to be able to exclude sites from form autofill".
Comment 20 VanillaMozilla 2007-09-11 12:32:13 PDT
Hoping that each Web site turns off autocompletion doesn't seem like adequate protection for the user.  This bug is worth reconsidering, as Firefox makes a very convenient key logger, in effect.

On the other hand, if this is an option, what's to prevent a thief from disabling the option without the user's knowledge?  I don't see how you can provide strong protection against autocompletion as long as protection is optional and can be turned off.
Comment 21 Robert Rothenberg 2007-09-12 02:47:25 PDT
(In reply to comment #20)

> On the other hand, if this is an option, what's to prevent a thief from
> disabling the option without the user's knowledge?  I don't see how you can
> provide strong protection against autocompletion as long as protection is
> optional and can be turned off.

Once a thief gets access to your machine, it becomes a moot point.

This isn't a request for "strong protection against autocompletion" (whatever that is).  It's simply a request to make Firefox smart enough to not save autocompletion for secure (https) pages, so that users can enjoy the benefits of autocompletion without it saving their passwords or credit card numbers.

Comment 22 Jesse Jones 2009-05-31 15:29:28 PDT
(In reply to comment #21)
> (In reply to comment #20)
> 
> > On the other hand, if this is an option, what's to prevent a thief from
> > disabling the option without the user's knowledge?  I don't see how you can
> > provide strong protection against autocompletion as long as protection is
> > optional and can be turned off.
> 
> Once a thief gets access to your machine, it becomes a moot point.
> 
> This isn't a request for "strong protection against autocompletion" (whatever
> that is).  It's simply a request to make Firefox smart enough to not save
> autocompletion for secure (https) pages, so that users can enjoy the benefits
> of autocompletion without it saving their passwords or credit card numbers.

Right; the idea is to eliminate "opportunism".  If my laptop is stolen, Firefox's current behavior makes it easy for a thief to find a https: site in my history, go to it, check out, and then just let autocomplete hand them my complete credit card details.  Heck, I've had autocomplete offer my credit card details on sites I've never shopped on before; I'm assuming it sees the same field name and populates it appropriately - so a thief wouldn't even need to check my history, but just go to any shopping site and they'd have my info.
Comment 23 Justin Dolske [:Dolske] 2009-07-23 12:48:13 PDT
Given the interest and relative ease of implementing this, I'd take a patch should someone write one.

...At least for the backend support (ie, adding a pref that can be changed in about:config). I think adding a checkbox to the main Preferences UI will be a tough sell, though that's not my call. In any case, that can be dealt with as a separate bug once form storage supports the pref.
Comment 24 Matthew N. [:MattN] (behind on reviews) 2009-07-25 00:59:27 PDT
*** Bug 505954 has been marked as a duplicate of this bug. ***
Comment 25 Matthias Versen [:Matti] 2010-01-29 08:14:42 PST
*** Bug 542986 has been marked as a duplicate of this bug. ***
Comment 26 Chris Ross 2010-01-29 11:45:48 PST
(In reply to comment #25)
> *** Bug 542986 has been marked as a duplicate of this bug. ***

Sorry for the duplicate: I did search but nothing showed up (too old...). 
For what it's worth I have re-modified the statute of my duplicate from 'Resolved' (because it isn't) to 'Verified'.

I am shocked and astonished that this issue has never been taken seriously.
OF COURSE I appreciate the software and the enormous team effort for which I am extremely grateful: however this is not a bug (in the conventional sense) but a VERY serious security loophole and DESIGN FAULT (that one does not expect to see other than in Microsoft products).
For the first time since using SeaMonkey and Mozilla (from the start - after Netscape) it has shaken my confidence.

 - OF COURSE data security should not depend on the design of a web page.
 - OF COURSE secure page data should not be stored unencrypted (without at least asking confirmation from the user).

These are basic, fundamental principles which are being violated and ignored.

I am broadcasting a recommendation to all my clients and contacts using SeaMonkey or FireFox, as well as in the relevant forums, to disable autocompletion as long as this defect has not been corrected, as well as to delete the files concerned (and I will do these myself on any computer on which comes through my workshop).
(I will continue using it myself to monitor the flaw by regular checks on the data saved.)

Regards (and thanks to all contributors anyway)
Christophe (programmer and computer engineer)
Comment 27 Matthias Versen [:Matti] 2010-02-05 06:03:22 PST
*** Bug 544369 has been marked as a duplicate of this bug. ***
Comment 28 Chris Ross 2010-02-14 14:19:39 PST
*** Bug 542008 has been marked as a duplicate of this bug. ***
Comment 29 Eddy Ferreira 2010-03-16 17:39:08 PDT
Created attachment 432965 [details] [diff] [review]
Patch to add requested functionality

This patch implements the requested feature--saving of form history is disabled on https pages. It is controlled by browser.formfill.disable_on_https which is true by default.

As I am new to the Mozilla codebase and this is my first attempt at a patch, I still have to figure out how to write test cases for it... any help or suggestions would be much appreciated.
Comment 30 Justin Dolske [:Dolske] 2010-03-16 19:59:01 PDT
Comment on attachment 432965 [details] [diff] [review]
Patch to add requested functionality

Oh boy, a patch! Fantastic!

>+pref("browser.formfill.disable_on_https", true);

At risk of bikeshedding... It would be better to have a pref name that avoids double negatives (eg, disable=false), and is clearer that it only prevents *saving* https data.

So, maybe .save_https_forms or something like that?

The default value for the pref should be to save such data (same as today).

>--- a/toolkit/components/satchel/src/nsStorageFormHistory.cpp
>+nsFormHistory::InitPrefs()
>+{
>+    nsCOMPtr<nsIPrefService> prefService =
>+        do_GetService(NS_PREFSERVICE_CONTRACTID);
>+    if (!prefService)
>+      return NS_ERROR_FAILURE;

NS_ENSURE_TRUE(prefService, NS_ERROR_FAILURE);

Ditto for the other cases of this error-checking pattern.

>+    branchInternal->AddObserver(PREF_FORMFILL_ENABLE,
>+                                gFormHistory, PR_TRUE);
>+    branchInternal->AddObserver(PREF_FORMFILL_DISABLE_ON_HTTPS,
>+                                gFormHistory, PR_TRUE);

Instead of adding an observer for each pref, just have a single call with an empty string for the first arguemnt. This causes changes for any pref name in that branch to send a notification.


>+    nsresult rv;
>+    nsCOMPtr<nsIContent> formCont = do_QueryInterface(formElt, &rv);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+    nsCOMPtr<nsIDocument> doc;
>+    doc = formCont->GetOwnerDoc();
>+    if (doc) {
>+      nsIURI *docURI = doc->GetDocumentURI();
>+      if (docURI) {

Instead of nesting here, just do NS_ENSURE_TRUE(doc, NS_OK) (same for docURI). I suspect these can never be null anyway, but I'll be paranoid until we get around to doing a JS rewrite of this file. :)

r- for these small nits, but for your first patch this is well done!

Oh, and we'll also need tests. I'll reply to that in a separate comment.
Comment 31 Justin Dolske [:Dolske] 2010-03-16 20:12:17 PDT
(In reply to comment #29)

> As I am new to the Mozilla codebase and this is my first attempt at a patch, I
> still have to figure out how to write test cases for it... any help or
> suggestions would be much appreciated.

To test the changes in this patch, there just need to be a few additions to the existing tests in /toolkit/components/satchel/test/test_form_submission.html.

See https://developer.mozilla.org/en/Mochitest for some details on how to write and run such tests. You might also look at the patch for bug 188285, which is also adding some simple tests here.

The two things to tests are (1) having an SSL action URL for an <input> on a non-SSL page, and (2) having a non-SSL action for an <input> on an SSL page.

Oh. Hmm. The second case will be a little more difficult to write, so it would have to be done in a iframe... I'll make you a deal; if you write the test for the first case I'll take care of the second?
Comment 32 Eddy Ferreira 2010-03-16 20:59:28 PDT
Sounds good--I think I can write the first test case without undue strain. The second case is what seemed more daunting, since I couldn't find any examples of opening SSL pages for testing (maybe if I took a closer look at the tests for security/manager/ssl I'd find something?)

I'll write something to test the first case soon, and address the things you pointed out in the patch. One note about that though--

> Instead of nesting here, just do NS_ENSURE_TRUE(doc, NS_OK) (same for docURI).
> I suspect these can never be null anyway, but I'll be paranoid until we get
> around to doing a JS rewrite of this file. :)

This'll change the semantics. At the moment, if doc or docURI is null, it just proceeds with saving the form data. I didn't know if they'd ever be null, but my reasoning was that if for some reason we got a form that doesn't live in a document, or a document that doesn't have a URI, then we don't have to worry about it being an HTTPS page so we should just keep going. If they really won't ever be null, then I suppose it doesn't matter one way or the other. So I'll rewrite it your way.
Comment 33 Justin Dolske [:Dolske] 2010-03-17 17:05:22 PDT
(In reply to comment #32)

> > Instead of nesting here, just do NS_ENSURE_TRUE(doc, NS_OK)
> 
> This'll change the semantics. At the moment, if doc or docURI is null, it just
> proceeds with saving the form data.

That's fine, if this case can somehow happen we can always fix it later (if it's a case we even want to handle).
Comment 34 Justin Dolske [:Dolske] 2010-03-30 19:37:08 PDT
Eddy, are you going to be able to finish this patch up soon? We're about to make some invasive changes to this code, and I want to make sure pending patches have a change to get in before we break everything. :)
Comment 35 Eddy Ferreira 2010-03-31 00:53:08 PDT
Ok, I'll put aside some time over the next couple days and get it in. My schedule was really packed over the last couple weeks but it's settling down now so I'll have time to do it. How's Friday sound?
Comment 36 Justin Dolske [:Dolske] 2010-03-31 14:49:08 PDT
That would be fantastic.
Comment 37 Eddy Ferreira 2010-04-02 21:47:38 PDT
Created attachment 436837 [details] [diff] [review]
Patch v2

All right, here's what I've got.
Comment 38 Justin Dolske [:Dolske] 2010-04-06 18:39:53 PDT
Comment on attachment 436837 [details] [diff] [review]
Patch v2

Looks good!
Comment 39 Justin Dolske [:Dolske] 2010-04-06 18:42:18 PDT
Created attachment 437478 [details] [diff] [review]
Patch v.3

Updated with a few formatting nits, and added tests for submitting from an https:// URL.
Comment 40 Justin Dolske [:Dolske] 2010-04-08 01:23:22 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/f5dcc84e887c

Thanks for the patch!

This should be in tomorrow's trunk nightly (if I squeaked in under the wire), and will be in the Developer Preview Alpha 5 which should be cut in a couple weeks.
Comment 41 Chris Ross 2011-06-23 10:23:02 PDT
Much later (have been using 'Autofill forms' with native autocompletion off...)

This bug is *** NOT FIXED ***
(Verified with SeaMonkey 2.0.14)

I now have a recent list of 4 https: sites for which credit card numbers and CV2 values are saved.
(Of course I have browser.formfill.disable_on_https=true)
I have verified (where possible) that some of the fields concerned also have "autocomplete=off": I will verify the others on next access.

(Will supply details - URL's and field names - on request by mail perso.)

Please re-open...
Comment 42 Jesse Ruderman 2011-06-23 13:45:21 PDT
Chris, please file a new bug report. You can mention the bug number here.
Comment 43 Chris Ross 2011-06-23 15:14:53 PDT
OK, Jesse, thanks :-)

New Bug filed: 666773

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