New issue
Advanced search Search tips

Issue 908385 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 28
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Password manager can save an empty password

Project Member Reported by vabr@chromium.org, Nov 26

Issue description

Chrome Version: 70.0.3538.76
OS: Chrome (but likely others)

What steps will reproduce the problem?
(1) Go to https://www.mvg.de/kundenportal/profil/registrieren.html
(2) Focus the first password field, ask Chrome to generate a password (e.g., through the context menu)
(3) Accept the generated password
(4) Search chrome://settings/passwords for passwords on mvg.de

What is the expected result?
All passwords found are non-empty.

What happens instead?
The entry pre-saved during step (3) has an empty password value.

When evolving the password manager code, we have been assuming for quite some time that empty passwords are never saved. Storing them might cause unexpected breakages (plus, it is not useful to the user).

(This is related to bug 908268 in that it happens at the same time, but it might require a separate mitigation to ensure that we never save empty passwords.)
 
Cc: vasi...@chromium.org
Why do we store an empty password instead of the generated one?
The exact problem why an empty password is saved on generation, is that parsing (old parsing) fails, it doesn't take into account that the password field should be generated field (in this case confirmation field, since it was incorrectly chosen). Apparently for case of pre-saving, there are no checks on password non-emptiness.
Labels: -Pri-3 M-71 Pri-2
Is this happening for generated passwords only? (At least I could not reproduce this manually without using password generation).

Is this cause by the code that was meant to handle manual alterations of a generated password after it was filled?

Can this bug cause crashes in any code that assumes passwords to be non-empty? (in which case I'd suggest to further increase the priority)
Cc: -vasi...@chromium.org
Owner: vasi...@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c355fbe49fceeea48528a4c42dcfc6f765d54a9

commit 3c355fbe49fceeea48528a4c42dcfc6f765d54a9
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Wed Nov 28 16:23:26 2018

Prevent manual password generation for the case when it's broken.

It doesn't work correctly for the password fields without name and id. In this
case Chrome should not allow the manual password generation to start. Automatic
password generation should not trigger on the problematic pages anyway.

Bug:  908385 
Change-Id: Ic6611e01f99ad9e4052a4198ba5f9f13217d6dd6
Reviewed-on: https://chromium-review.googlesource.com/c/1352317
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611723}
[modify] https://crrev.com/3c355fbe49fceeea48528a4c42dcfc6f765d54a9/chrome/renderer/autofill/password_generation_agent_browsertest.cc
[modify] https://crrev.com/3c355fbe49fceeea48528a4c42dcfc6f765d54a9/components/autofill/content/renderer/password_generation_agent.cc
[modify] https://crrev.com/3c355fbe49fceeea48528a4c42dcfc6f765d54a9/components/password_manager/core/browser/form_saver_impl.cc
[modify] https://crrev.com/3c355fbe49fceeea48528a4c42dcfc6f765d54a9/components/password_manager/core/browser/password_manager_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment