Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Don't allow hidden fields to be populated with autofill data to avoid autofill phishing #6558

Closed
cezaraugusto opened this issue Jan 6, 2017 · 14 comments
Closed

Don't allow hidden fields to be populated with autofill data to avoid autofill phishing #6558

cezaraugusto opened this issue Jan 6, 2017 · 14 comments
Assignees
@darkdh
Labels
feature/autofill needs-mockup A feature which needs design mockup to be implemented. open-in-brave-core QA/test-plan-specified security

Comments

@cezaraugusto
Copy link
Contributor

cezaraugusto commented Jan 6, 2017 •
edited by luixxiul
Loading

Test plan

#12835 (comment)

Did you search for similar issues before submitting this one?
yes

Describe the issue you encountered:
If you use autofill data to fill in a form with hidden fields, they get populated either.

This is a security risk since unaware users can give their information to a malicious website.

Expected behavior:

We shouldn't autofill data for any type of hidden fields

  • Platform (Win7, 8, 10? macOS? Linux distro?): n/a

  • Brave Version (revision SHA): n/a

  • Steps to reproduce:

    1. Have all autofill data filled
    2. Go to this site: https://anttiviljami.github.io/browser-autofill-phishing/
    3. Autofill fields
    4. Open devTools > Network > Click on the .html file > Headers tab
    5. Form data was all filled even if you can only see name/email

  • Screenshot if needed:

(Originaly from this site)

autofill-demo

  • Any related issues: n/a

/cc @diracdeltas
@diracdeltas diracdeltas changed the title Don't allow hidden fields to be populated with autofill data to avoid autofill pishing Don't allow hidden fields to be populated with autofill data to avoid autofill phishing Jan 10, 2017
@diracdeltas diracdeltas added this to the 0.13.1 milestone Jan 10, 2017
@diracdeltas
Copy link
Member

this has some usability cost, but i think ideally we would not autofill until the field is manually focused by the user (not by javascript). @darkdh could you look into this?

@bsclifton bsclifton modified the milestones: 0.13.2, 0.13.1 Jan 10, 2017
@bsclifton
Copy link
Member

Bumping back to 0.13.2 for now (just since there are so many items stacked up)- but definitely welcome in 0.13.1 if finished early 😄

@darkdh
Copy link
Member

darkdh commented Jan 11, 2017

Sure. I will take it

@darkdh darkdh self-assigned this Jan 11, 2017
@bbondy bbondy modified the milestones: 0.13.5, 0.13.6 Feb 15, 2017
@darkdh
Copy link
Member

darkdh commented Jul 19, 2017

per discussion with @diracdeltas and @bridiver , our possible solutions are

  1. require manual focus on field to autofill
  2. show the values are about to be filled in context menu
  3. try to detect user tricking fields

(1) might be too annoying for users to fill normal forms
(3) There are many ways to trick users not only limited to hidden element but also low opacity, setting text and background colors, etc…

We came to decision of (2) would be better among these 3.

@bradleyrichter , we might need some inputs from you.

cc @diracdeltas , @bridiver for adding what I might missed

@bsclifton bsclifton added the needs-mockup A feature which needs design mockup to be implemented. label Jul 19, 2017
@bsclifton bsclifton added this to the Triage Backlog milestone Nov 27, 2017
@darkdh darkdh modified the milestones: Triage Backlog, 0.21.x (Developer Channel) Jan 3, 2018
@jumde jumde removed the sec-low label Jan 3, 2018
@diracdeltas
Copy link
Member

removing sec-low because this has been in the news recently (for password autofill): https://freedom-to-tinker.com/2017/12/27/no-boundaries-for-user-identities-web-trackers-exploit-browser-login-managers/

@diracdeltas
Copy link
Member

We re-discussed and decided (1) require manual focus on field to autofill is probably the best experience right now. It matches FF's autofill behavior and doesn't require the user to manually review the fields about to be filled in the context menu to make sure there is nothing unexpected.

@darkdh
Copy link
Member

darkdh commented Jan 3, 2018

#6558 (comment)
will be addressed here
#12489

@NejcZdovc
Copy link
Contributor

@darkdh can this issue be closed based on #12489 ?

@darkdh
Copy link
Member

darkdh commented Jan 4, 2018

@NejcZdovc , no because this one requires different fix

darkdh added a commit to brave/muon that referenced this issue Jan 4, 2018
@darkdh
Autofill the fields when focused
0de85e6 
fix brave/browser-laptop#6558

Auditors: @bridiver, @diracdeltas, @bbondy
@darkdh darkdh mentioned this issue Jan 4, 2018
Closed
@darkdh
Copy link
Member

darkdh commented Jan 23, 2018 •
edited
Loading

kapture 2018-01-24 at 15 33 28

work progress updated

darkdh added a commit to brave/muon that referenced this issue Jan 24, 2018
@darkdh
Ask user consent before autofill
730aa2d 
fix brave/browser-laptop#6558

Auditors: @bridiver, @diracdeltas, @bbondy
darkdh added a commit that referenced this issue Jan 24, 2018
@darkdh
Browser-laptop changes for trigger ask user consent before autofill
0d52b9b 
fix #6558

Auditors: @bridiver, @diracdeltas, @bbondy

Test Plan:
1. Make sure you have autofill profile in about:autofill
2. Go to https://anttiviljami.github.io/browser-autofill-phishing/
3. Click name field to trigger autofill
4. Select one suggestion
5. There will be a dialog informs users the values about to auotfill
6. If user click ok, the values will be filled.
   If user click cancel, no values will be filled.
@darkdh darkdh mentioned this issue Jan 24, 2018
Closed
10 tasks
darkdh added a commit that referenced this issue Jan 24, 2018
@darkdh
Browser-laptop changes for trigger ask user consent before autofill
bbfa1d7 
fix #6558

Auditors: @bridiver, @diracdeltas, @bbondy

Test Plan:
1. Make sure you have autofill profile in about:autofill
2. Go to https://anttiviljami.github.io/browser-autofill-phishing/
3. Click name field to trigger autofill
4. Select one suggestion
5. There will be a dialog informs users the values about to auotfill
6. If user click ok, the values will be filled.
   If user click cancel, no values will be filled.
@darkdh darkdh modified the milestones: 0.21.x (Beta Channel), 0.22.x (Developer Channel) Feb 19, 2018
@bbondy bbondy modified the milestones: 0.22.x (Developer Channel), 0.23.x (Nightly Channel) Feb 25, 2018
@bsclifton bsclifton modified the milestones: 0.23.x (Nightly Channel), Completed work Feb 28, 2018
@bsclifton
Copy link
Member

@jumde do you have a link to this in brave-core? I can't find a matching issue

@bsclifton bsclifton removed this from the Completed work milestone Sep 1, 2018
@darkdh
Copy link
Member

darkdh commented Sep 3, 2018 via email

@jumde
Copy link
Contributor

jumde commented Sep 4, 2018

@darkdh @bsclifton this is a problem in brave-core as well, that's why I added the open-in-brave-core label to it. I'll follow-up with @tomlowenthal to figure out how we are tracking other security and privacy issues with open-in-brave-core label.

@jumde jumde mentioned this issue Sep 4, 2018
Open
@jumde
Copy link
Contributor

jumde commented Sep 4, 2018

In brave-core: brave/brave-browser#949 cc: @bsclifton

@jumde jumde closed this as completed Sep 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@darkdh darkdh

Labels
feature/autofill needs-mockup A feature which needs design mockup to be implemented. open-in-brave-core QA/test-plan-specified security
Projects
None yet
Milestone
No milestone
8 participants
@diracdeltas @bbondy @jumde @luixxiul @cezaraugusto @bsclifton @NejcZdovc @darkdh