New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 51727 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Sep 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Security
M-7

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

autocomplete entries submitted by javascript should not be stored in db (similar to autofill bug 48225)

Project Member Reported by infe...@chromium.org, Aug 10 2010

Issue description

subject says it all. we dont want auto submitted javascript to submit spam entries in a form field. that was fixed for autofill ( bug 48225 ), but i still see it in autocomplete entries.

While we do restrict no of entries per autocomplete field (http://jeremiahgrossman.blogspot.com/2010/07/in-firefox-we-cant-read-auto-complete.html), we shouldn't be storing any one at all since it can kick off legitimate entries.
 
Status: WillMerge
Needs to be merged to 472 and 375.

Comment 3 by bugdro...@gmail.com, Aug 10 2010

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=55626 

------------------------------------------------------------------------
r55626 | inferno@chromium.org | 2010-08-10 15:39:30 -0700 (Tue, 10 Aug 2010) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/autocomplete_history_manager.cc?r1=55626&r2=55625

Not store autocomplete entries in DB for forms submitted using javascript.

BUG= 51727 

Review URL: http://codereview.chromium.org/3149003
------------------------------------------------------------------------

Comment 4 by lcam...@gmail.com, Aug 10 2010

Out of curiosity, do we care about auto-complete pollution on user-initiated submits?

(i.e., I can have a form with 1000 off-screen fields)
@lcamtuf: nothing serious in this one. just submitted patch for synchronous behavior with autofill. also, i remember if we have 1000 fields with same name on the same page (and we ask user to submit a button), we just store data from the first field. so, we pollute one entry in a particular field(name) at a time from user initiated behavior. 
let me double check this.

Comment 7 by bugdro...@gmail.com, Aug 10 2010

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=55632 

------------------------------------------------------------------------
r55632 | inferno@chromium.org | 2010-08-10 16:19:17 -0700 (Tue, 10 Aug 2010) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/autocomplete_history_manager_unittest.cc?r1=55632&r2=55631

Unittest fix. Need to put usersubmitted = true for unittests.

BUG= 51727 

Review URL: http://codereview.chromium.org/3151006
------------------------------------------------------------------------

@lcamtuf:: the situation is a little ugly here.

for autofill, there is no issue since we use only one field from form per name. so, when we persuade user to click the submit button (other fields can be hidden with css), only one spam profile gets created.

for autocomplete, situation is ugly. we can have multiple fields in the same form with one name and different ids. autocomplete history manager will store them seperately. So, effectively, we could achieve the same entry spamming by having multiple fields per form (with same name, different id) like 1000 and just ask user to click submit. we cap the no of the entries, but they can still be overwritten. this is a headache, but i dont see any fix possible for this.

Comment 9 by lcam...@gmail.com, Aug 11 2010

How about capping the number of text fields per page (or better, per origin) that we add to the autocomplete cache, with FIFO pruning or so?


autocomplete entries are not origin specific and are available for use on all sites, so we cannot use origin for tracking. we currently cap the no of entries available for a particular form field (name specific). lets brainstorm the capping of no of text fields per page. I need to think more on how this can be useful. From an attacker perspective, he or she might want to pollute data in generic fields like name, email, phone, so limiting the no of text fields per page (so something like 1000) will still be able to pollute data in those fields.

James, David - any thoughts. this will be m7, but good to get your thoughts whether this could be fixed or not. like autofill, i dont know if we can just store 1 form field per name (and ignore rest of entries with same name) without breaking any normal behavior.
Labels: Mstone-6
Status: Available
Just a fyi, my changes are merged to 472. So javascript spamming vector is gone. (Bugdroid looks down)

I had a chat with David on this and this is what i purpose to fix the user click on a button vector.

1) only use the first field for a particular name.
2) cap no of fields per form (have to be cautious on this number, i dont expect any real world site to have more than 256 form fields e.g.).
Status: Started
Status: Fixed
The user-click fix is in at http://src.chromium.org/viewvc/chrome?view=rev&revision=55781.  (Not sure why bugdroid is not updating this bug report...).

I'm not planning to merge this fix to 472, since it is M7.
Status: WillMerge
Fix is simple enough, will wait a week to bake on dev channel before merging to v6.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=55662 

------------------------------------------------------------------------
r55662 | inferno@chromium.org | 2010-08-10 20:31:01 -0700 (Tue, 10 Aug 2010) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/472/src/chrome/browser/autocomplete_history_manager_unittest.cc?r1=55662&r2=55661

Merge 55632 - Unittest fix. Need to put usersubmitted = true for unittests.

BUG= 51727 

Review URL: http://codereview.chromium.org/3151006

TBR=inferno@chromium.org
Review URL: http://codereview.chromium.org/3110007
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=55661 

------------------------------------------------------------------------
r55661 | inferno@chromium.org | 2010-08-10 20:29:40 -0700 (Tue, 10 Aug 2010) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/472/src/chrome/browser/autocomplete_history_manager.cc?r1=55661&r2=55660

Merge 55626 - Not store autocomplete entries in DB for forms submitted using javascript.

BUG= 51727 

Review URL: http://codereview.chromium.org/3149003

TBR=inferno@chromium.org
Review URL: http://codereview.chromium.org/3145007
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=55781 

------------------------------------------------------------------------
r55781 | dhollowa@chromium.org | 2010-08-11 14:03:29 -0700 (Wed, 11 Aug 2010) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/webdata/web_database.cc?r1=55781&r2=55780
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/webdata/web_database.h?r1=55781&r2=55780
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/webdata/web_database_unittest.cc?r1=55781&r2=55780

Autocomplete entries submitted are limited in number.

Limits the number of Autocomplete entries added to the WebDB, per form submission, to a maximum of 256.  If elements occur that have duplicate names, only the first occurrence is added.

BUG= 51727 
TEST=WebDatabaseTest.Autofill_AddFormFieldValues

Review URL: http://codereview.chromium.org/3143005
------------------------------------------------------------------------

Status: Fixed
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=55894 

------------------------------------------------------------------------
r55894 | inferno@chromium.org | 2010-08-12 09:49:29 -0700 (Thu, 12 Aug 2010) | 11 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/472/src/chrome/browser/webdata/web_database.cc?r1=55894&r2=55893
   M http://src.chromium.org/viewvc/chrome/branches/472/src/chrome/browser/webdata/web_database.h?r1=55894&r2=55893
   M http://src.chromium.org/viewvc/chrome/branches/472/src/chrome/browser/webdata/web_database_unittest.cc?r1=55894&r2=55893

Merge 55781 - Autocomplete entries submitted are limited in number.

Limits the number of Autocomplete entries added to the WebDB, per form submission, to a maximum of 256.  If elements occur that have duplicate names, only the first occurrence is added.

BUG= 51727 
TEST=WebDatabaseTest.Autofill_AddFormFieldValues

Review URL: http://codereview.chromium.org/3143005

TBR=dhollowa@chromium.org
Review URL: http://codereview.chromium.org/3169008
------------------------------------------------------------------------

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=56469 

------------------------------------------------------------------------
r56469 | rsimha@chromium.org | 2010-08-17 17:45:26 -0700 (Tue, 17 Aug 2010) | 15 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/live_sync/two_client_live_autofill_sync_test.cc?r1=56469&r2=56468
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/live_sync/two_client_live_preferences_sync_test.cc?r1=56469&r2=56468

Fix failing sync integration tests.

TwoClientLivePreferencesSyncTest.Security was failing due to a couple of
EXPECT_NEs that ought to have been EXPECT_EQs.

TwoClientLiveAutofillSyncTest.Client1HasData was failing due to a change
in autofill behavior introduced in r55781 to fix security bug  crbug.com/51727 .

This changelist also includes a clarification in the logic used by
TwoClientLiveAutofillSyncTest.BothHaveData.

BUG= 51727 , 51956 
TEST=sync_integration_tests

Review URL: http://codereview.chromium.org/3169019
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=58038 

------------------------------------------------------------------------
r58038 | inferno@chromium.org | 2010-08-31 12:16:51 -0700 (Tue, 31 Aug 2010) | 11 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/472/src/chrome/browser/autocomplete_history_manager.cc?r1=58038&r2=58037

Revert 55661 - Merge 55626 - Not store autocomplete entries in DB for forms submitted using javascript.

BUG= 51727 , 52940 

Review URL: http://codereview.chromium.org/3149003

TBR=inferno@chromium.org
Review URL: http://codereview.chromium.org/3145007

TBR=inferno@chromium.org
Review URL: http://codereview.chromium.org/3286006
------------------------------------------------------------------------

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=58039 

------------------------------------------------------------------------
r58039 | inferno@chromium.org | 2010-08-31 12:18:11 -0700 (Tue, 31 Aug 2010) | 11 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/472/src/chrome/browser/autocomplete_history_manager_unittest.cc?r1=58039&r2=58038

Revert 55662 - Merge 55632 - Unittest fix. Need to put usersubmitted = true for unittests.

BUG= 51727 , 52940 

Review URL: http://codereview.chromium.org/3151006

TBR=inferno@chromium.org
Review URL: http://codereview.chromium.org/3110007

TBR=inferno@chromium.org
Review URL: http://codereview.chromium.org/3276006
------------------------------------------------------------------------

Labels: -Mstone-6 Mstone-7
Status: Assigned
Reopening pending the issue that came up in bug http://code.google.com/p/chromium/issues/detail?id=52940. Note that David's fix part is not reverted. Also, only changes to 472 are reverted. Trunk needed to be fixed.
Labels: Restrict-View-SecurityTeam
Labels: -Restrict-View-SecurityTeam
Status: Started
Status: Fixed
WebKit fix submitted.  Marking this bug fixed, pending WebKit CL landing and roll).

https://bugs.webkit.org/show_bug.cgi?id=45128


Labels: OS-All
Status: FixUnreleased
Labels: -Restrict-View-SecurityNotify
Status: Fixed
Labels: Type-Security
Labels: SecImpacts-Stable
Batch update.
Project Member

Comment 34 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 35 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Area-Internals -SecSeverity-Low -Mstone-7 -Type-Security -SecImpacts-Stable Security-Severity-Low Security-Impact-Stable M-7 Cr-Internals Type-Bug-Security
Project Member

Comment 36 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

Comment 37 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-Low Security_Severity-Low
Project Member

Comment 38 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 39 by sheriffbot@chromium.org, Oct 1 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 40 by sheriffbot@chromium.org, Oct 2 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic

Sign in to add a comment