FATAL:password_manager.cc(799)] Check failed: PasswordForm::SCHEME_HTML != preferred_match.scheme (0 vs. 0) |
|||||
Issue descriptionVersion: 54.0.2839.0 (Developer Build) (64-bit) with dcheck_always_on=1 OS: Linux What steps will reproduce the problem? (1) https://d4u.jp/admin/ (2) (3) What is the expected output? What do you see instead? browser crashed FATAL:password_manager.cc(799)] Check failed: PasswordForm::SCHEME_HTML != preferred_match.scheme (0 vs. 0) (gdb) bt #0 0x00007fffeea76c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007fffeea7a028 in __GI_abort () at abort.c:89 #2 0x00007ffff7a6dec2 in base::debug::BreakDebugger() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #3 0x00007ffff7a94ada in logging::LogMessage::~LogMessage() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #4 0x000055555690a99e in password_manager::PasswordManager::AutofillHttpAuth(std::map<std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> >, autofill::PasswordForm const*, std::less<std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > >, std::allocator<std::pair<std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > const, autofill::PasswordForm const*> > > const&, autofill::PasswordForm const&) const () #5 0x0000555556926e11 in password_manager::PasswordFormManager::ProcessLoginPrompt() () #6 0x0000555556926517 in password_manager::PasswordFormManager::ProcessMatches(std::vector<autofill::PasswordForm const*, std::allocator<autofill::PasswordForm const*> > const&, unsigned long) () #7 0x00005555569316a4 in password_manager::FormFetcherImpl::SetResults(std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >) () #8 0x000055555692710a in password_manager::PasswordFormManager::OnGetPasswordStoreResults(std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >) () #9 0x0000555556912253 in void base::internal::FunctorTraits<void (password_manager::PasswordStoreConsumer::*)(std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >), void>::Invoke<base::WeakPtr<password_manager::PasswordStoreConsumer> const&, std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > > >(void (password_manager::PasswordStoreConsumer::*)(std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >), base::WeakPtr<password_manager::PasswordStoreConsumer> const&, std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >&&) () #10 0x000055555691217f in void base::internal::Invoker<base::internal::BindState<void (password_manager::PasswordStoreConsumer::*)(std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >), base::WeakPtr<password_manager::PasswordStoreConsumer>, base::internal::PassedWrapper<std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > > > >, void ()>::RunImpl<void (password_manager::PasswordStoreConsumer::* const&)(std::vector<s td::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >), std::tuple<base::WeakPtr<password_manager::PasswordStoreConsumer>, base::internal::PassedWrapper<std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > > > > const&, 0ul, 1ul>(void (password_manager::PasswordStoreConsumer::* const&)(std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > >), std::tuple<base::WeakPtr<password_manager::PasswordStoreConsumer>, base::internal::PassedWrapper<std::vector<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> >, std::allocator<std::unique_ptr<autofill::PasswordForm, std::default_delete<autofill::PasswordForm> > > > > > const&, base::IndexSequence<0ul, 1ul>) () #11 0x00007ffff7a74796 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask const&) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #12 0x00007ffff7a9f105 in base::MessageLoop::RunTask(base::PendingTask const&) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #13 0x00007ffff7a9f468 in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #14 0x00007ffff7a9f81b in base::MessageLoop::DoWork() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #15 0x00007ffff7aa181a in base::(anonymous namespace)::WorkSourceDispatch(_GSource*, int (*)(void*), void*) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #16 0x00007ffff1671e04 in g_main_dispatch (context=0x321dbb80ac00) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3064 #17 g_main_context_dispatch (context=context@entry=0x321dbb80ac00) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3663 #18 0x00007ffff1672048 in g_main_context_iterate ( context=context@entry=0x321dbb80ac00, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3734 #19 0x00007ffff16720ec in g_main_context_iteration (context=0x321dbb80ac00, may_block=1) at /build/buildd/glib2.0-2.40.2/./glib/gmain.c:3795 #20 0x00007ffff7aa1576 in base::MessagePumpGlib::Run(base::MessagePump::Delegate*) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #21 0x00007ffff7a9ec01 in base::MessageLoop::RunHandler() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #22 0x00007ffff7acd090 in base::RunLoop::Run() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libbase.so #23 0x000055555603c8ba in ChromeBrowserMainParts::MainMessageLoopRun(int*) () #24 0x00007ffff55a1d99 in content::BrowserMainLoop::RunMainMessageLoopParts() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libcontent.so #25 0x00007ffff55a5488 in content::BrowserMainRunnerImpl::Run() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libcontent.so #26 0x00007ffff559d18e in content::BrowserMain(content::MainFunctionParams const&) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libcontent.so #27 0x00007ffff5ea9cbe in content::RunNamedProcessTypeMain(std::string const&, content::MainFunctionParams const&, content::ContentMainDelegate*) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libcontent.so #28 0x00007ffff5eaa773 in content::ContentMainRunnerImpl::Run() () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libcontent.so #29 0x00007ffff5ea8fd0 in content::ContentMain(content::ContentMainParams const&) () from /usr/local/google/home/ukai/src/chromium-git/src/out.0/Release/./libcontent.so #30 0x0000555555a3546b in ChromeMain () #31 0x00007fffeea61f45 in __libc_start_main (main=0x555555a35420 <main>, argc=2, argv=0x7fffffffdb38, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffdb28) at libc-start.c:287 #32 0x0000555555a3533d in _start () Please use labels and text to provide additional information. https://chromium.googlesource.com/chromium/src/+/e0e1e99f39349e912612e58249393f2281de4d00
,
Aug 30 2016
ah, it wasn't https://d4u.jp/admin/, but http://d4u.jp/admin/
,
Aug 30 2016
Thanks, good to hear that SSL errors were not involved. I assume that the page had a HTML-based form before, where you saved a password, and switched to the HTTP auth instead, is that correct? My plan here is to modify PasswordFormManager::ProcessMatches to filter out all PasswordForm instances with a scheme not matching the observed_form_. It won't allow filling HTML forms into HTTP auth and vice versa, but that should not have been possible before either (not a secure way to handle credentials). Adding the security component as a FYI: Chrome apparently can fill HTML-based credentials into HTTP auth and vice versa (it just has a DCHECK against it). Will upload a patch soon.
,
Aug 31 2016
I don't remember it used a HTML-based form before...
,
Aug 31 2016
Thanks for your response. Just to avoid missing a different error, I'd like to verify what kind of password has Chrome stored for you on d4u.jp, in particular what is its PasswordForm::scheme and PasswordForm::action. Unfortunately, these properties are not visible through Chrome's UI, they need to be inspected in the backend storage. Could you please have a look for passwords stored on d4u.jp and report their action URL and scheme? You reported being on GNU/Linux, which can mean three different backends, so please follow the matching option below: (A) If your Chrome is using Gnome Keyring, try running "seahorse". Your password should be in the Default keyring. Just double-click the entry, select the "Details" tab, and read the values for scheme and action. (B) If your Chrome is using KDE's KWallet, you should be able to use "kwalletmanager" similarly to retrieve the scheme and action. (C) In all other cases, your passwords are stored in "Login Data" inside your profile directory. Locate the file using the profile directory path from about:version, close Chrome (use Ctrl+Shift+Q to ensure that no background process is running) and use sqlite3 to open the "Login Data" file. Then run the following SQL command to print the origins, actions and schemes of credentials on origins containing d4u: SELECT origin_url, action_url, scheme FROM logins WHERE origin_url LIKE '%d4u%'; Thanks! Vaclav
,
Aug 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da597b2f777ea4ac768164bee01f39a90fba3c65 commit da597b2f777ea4ac768164bee01f39a90fba3c65 Author: vabr <vabr@chromium.org> Date: Wed Aug 31 13:48:56 2016 Filter out credentials with non-matching schemes PasswordFormManager::ProcessMatches currently happily accepts credentials from PasswordStore with a different PasswordForm::Scheme than the observed form has. However, it still has a DCHECK against it later (in the Autofill* methods), so it is clearly not expecting these, rather than mixing the schemes being by design. And it should not be by design. Especially, if the saved credential is a non-HTML one, and should be filled in a HTML form. Mixing them makes the non-HTML credential vulnerable against (injected attacker's) JavaScript accessing them. This CL filters out credentials with non-matching scheme from the batch coming from the PasswordStore. Given the absence of DCHECKs in release builds, this actually changes the behaviour for Chrome users, but the change is a desired one. BUG= 640897 Review-Url: https://codereview.chromium.org/2298733002 Cr-Commit-Position: refs/heads/master@{#415622} [modify] https://crrev.com/da597b2f777ea4ac768164bee01f39a90fba3c65/components/password_manager/core/browser/password_form_manager.cc [modify] https://crrev.com/da597b2f777ea4ac768164bee01f39a90fba3c65/components/password_manager/core/browser/password_form_manager_unittest.cc
,
Aug 31 2016
The fix landed in #6. As dvadym@ pointed out in the review, this whole issue should be only restricted to Gnome and KDE. Everything using LoginDatabase should be unaffected. ukai@ -- after r415622 you should no longer hit the DCHECK. However, Chrome may also not fill all credentials it used to before (without DCHECK enabled). If you see any issues, have questions, or answer for me regarding #5, please update this bug. Thanks! Vaclav
,
Aug 31 2016
The CL got reverted, so reopening. Failing on https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/52229 with the attached log.
,
Aug 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/53105f2efbdccc7714b8bba189e628b0f87f7fba commit 53105f2efbdccc7714b8bba189e628b0f87f7fba Author: fgorski <fgorski@chromium.org> Date: Wed Aug 31 16:18:40 2016 Revert of Filter out credentials with non-matching schemes (patchset #1 id:1 of https://codereview.chromium.org/2298733002/ ) Reason for revert: Patch is crashing tests on windows bots. https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/52229 https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/52229/steps/browser_tests%20on%20Windows-7-SP1/logs/AutofillEditAddressWebUITest.testFieldValuesSaved Original issue's description: > Filter out credentials with non-matching schemes > > PasswordFormManager::ProcessMatches currently happily accepts credentials from > PasswordStore with a different PasswordForm::Scheme than the observed form has. > However, it still has a DCHECK against it later (in the Autofill* methods), so > it is clearly not expecting these, rather than mixing the schemes being by > design. > > And it should not be by design. Especially, if the saved credential is a > non-HTML one, and should be filled in a HTML form. Mixing them makes the > non-HTML credential vulnerable against (injected attacker's) JavaScript > accessing them. > > This CL filters out credentials with non-matching scheme from the batch coming > from the PasswordStore. Given the absence of DCHECKs in release builds, this > actually changes the behaviour for Chrome users, but the change is a desired > one. > > BUG= 640897 > > Committed: https://crrev.com/da597b2f777ea4ac768164bee01f39a90fba3c65 > Cr-Commit-Position: refs/heads/master@{#415622} TBR=mkwst@chromium.org,dvadym@chromium.org,vabr@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 640897 Review-Url: https://codereview.chromium.org/2298263002 Cr-Commit-Position: refs/heads/master@{#415648} [modify] https://crrev.com/53105f2efbdccc7714b8bba189e628b0f87f7fba/components/password_manager/core/browser/password_form_manager.cc [modify] https://crrev.com/53105f2efbdccc7714b8bba189e628b0f87f7fba/components/password_manager/core/browser/password_form_manager_unittest.cc
,
Sep 1 2016
re #5. ah, it was https://ssl.d4u.jp/admin/ and "Login Data" has sqlite> SELECT origin_url, action_url, scheme FROM logins WHERE origin_url LIKE '%d4u%'; https://ssl.d4u.jp/||0
,
Sep 1 2016
Thanks for the update, ukai@! Indeed, the "Login Data" record has a 0 in scheme, which means it should have been a HTML form. (Please let me know if you still think that credential from ssl.d4u.jp/admin has been saved as a HTTP auth, because that would mean another bug in Chromium.) Also, to correct my first paragraph in #7, Login Database is also affected (as evidenced by #9). The way the database retrieves logins ensures the URL scheme is the same (and this is true for all other backends), but not the PasswordForm::Scheme. This caused some confusion in https://codereview.chromium.org/2298733002/#msg12. Right now I am building on Windows to inspect why the bots failed yesterday, but the general issue seems to be understood.
,
Sep 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/209c98125433d97245ff70663ccb0698b3dc6aa9 commit 209c98125433d97245ff70663ccb0698b3dc6aa9 Author: vabr <vabr@chromium.org> Date: Thu Sep 01 16:32:30 2016 Test that LoginDatabase preserves PasswordForm::Scheme through migration When looking at http://crbug.com/640897 I was asking myself if a credential stored for a basic HTTP Auth could end up being stored with PasswordForm::SCHEME_HTML. So I added some tests to check this. They pass, so hopefully there is no issue with this in the production code. BUG= 640897 Review-Url: https://codereview.chromium.org/2291123008 Cr-Commit-Position: refs/heads/master@{#415968} [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/password_manager/core/browser/login_database_unittest.cc [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/password_manager/core/browser/login_database_win.cc [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v1.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v10.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v11.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v12.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v13.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v14.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v15.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v16.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v17.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v18.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v1_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v2.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v2_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v3.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v3_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v4.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v5.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v6.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v7.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v8.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v9.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v9_without_use_additional_auth_field.sql
,
Sep 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/209c98125433d97245ff70663ccb0698b3dc6aa9 commit 209c98125433d97245ff70663ccb0698b3dc6aa9 Author: vabr <vabr@chromium.org> Date: Thu Sep 01 16:32:30 2016 Test that LoginDatabase preserves PasswordForm::Scheme through migration When looking at http://crbug.com/640897 I was asking myself if a credential stored for a basic HTTP Auth could end up being stored with PasswordForm::SCHEME_HTML. So I added some tests to check this. They pass, so hopefully there is no issue with this in the production code. BUG= 640897 Review-Url: https://codereview.chromium.org/2291123008 Cr-Commit-Position: refs/heads/master@{#415968} [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/password_manager/core/browser/login_database_unittest.cc [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/password_manager/core/browser/login_database_win.cc [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v1.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v10.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v11.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v12.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v13.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v14.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v15.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v16.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v17.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v18.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v1_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v2.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v2_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v3.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v3_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v4.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v5.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v6.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v7.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v8.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v9.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v9_without_use_additional_auth_field.sql
,
Sep 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/209c98125433d97245ff70663ccb0698b3dc6aa9 commit 209c98125433d97245ff70663ccb0698b3dc6aa9 Author: vabr <vabr@chromium.org> Date: Thu Sep 01 16:32:30 2016 Test that LoginDatabase preserves PasswordForm::Scheme through migration When looking at http://crbug.com/640897 I was asking myself if a credential stored for a basic HTTP Auth could end up being stored with PasswordForm::SCHEME_HTML. So I added some tests to check this. They pass, so hopefully there is no issue with this in the production code. BUG= 640897 Review-Url: https://codereview.chromium.org/2291123008 Cr-Commit-Position: refs/heads/master@{#415968} [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/password_manager/core/browser/login_database_unittest.cc [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/password_manager/core/browser/login_database_win.cc [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v1.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v10.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v11.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v12.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v13.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v14.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v15.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v16.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v17.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v18.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v1_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v2.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v2_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v3.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v3_broken.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v4.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v5.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v6.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v7.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v8.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v9.sql [modify] https://crrev.com/209c98125433d97245ff70663ccb0698b3dc6aa9/components/test/data/password_manager/login_db_v9_without_use_additional_auth_field.sql
,
Sep 1 2016
Relanding of the fix (with an explanation of the bot failures) is at https://codereview.chromium.org/2306513002/.
,
Sep 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/639b09a35f402f1e49e0a9ef7cb72705abd96421 commit 639b09a35f402f1e49e0a9ef7cb72705abd96421 Author: vabr <vabr@chromium.org> Date: Mon Sep 05 12:38:02 2016 Reland "Filter out credentials with non-matching schemes" The original CL: https://codereview.chromium.org/2298733002 The revert: https://codereview.chromium.org/2298263002 The original CL caused failures on Win7 debug bots, which are not present in the CQ trybots set. The issue was genuine: the newly introduced code modified a vector and invalidated some iterators used later. The patch set 1 here is the reverted change, patch sets 2+ are fixes. The fix here also replaces "non-relevant" with "irrelevant", to improve the language, and does some further restructuring of the code based on reviewer feedback from dvadym@. Original description: Filter out credentials with non-matching schemes PasswordFormManager::ProcessMatches currently happily accepts credentials from PasswordStore with a different PasswordForm::Scheme than the observed form has. However, it still has a DCHECK against it later (in the Autofill* methods), so it is clearly not expecting these, rather than mixing the schemes being by design. And it should not be by design. Especially, if the saved credential is a non-HTML one, and should be filled in a HTML form. Mixing them makes the non-HTML credential vulnerable against (injected attacker's) JavaScript accessing them. This CL filters out credentials with non-matching scheme from the batch coming from the PasswordStore. Given the absence of DCHECKs in release builds, this actually changes the behaviour for Chrome users, but the change is a desired one. BUG= 640897 Review-Url: https://codereview.chromium.org/2306513002 Cr-Commit-Position: refs/heads/master@{#416536} [modify] https://crrev.com/639b09a35f402f1e49e0a9ef7cb72705abd96421/components/password_manager/core/browser/password_form_manager.cc [modify] https://crrev.com/639b09a35f402f1e49e0a9ef7cb72705abd96421/components/password_manager/core/browser/password_form_manager.h [modify] https://crrev.com/639b09a35f402f1e49e0a9ef7cb72705abd96421/components/password_manager/core/browser/password_form_manager_unittest.cc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by vabr@chromium.org
, Aug 29 2016Labels: Needs-Feedback Hotlist-Polish
Owner: vabr@chromium.org
Status: Assigned (was: Untriaged)