Fix a crash in ManagePasswordsState with use-after-free.

There are two bugs fixed together.
- HttpAuthManagerImpl::OnLoginSuccesful should care about the case when the
password store isn't done. In such a case the prompt should not be invoked.
- ManagePasswordsState should account for the method parameters may depend on
the internal memeber |form_manager_|.

Bug: 1017481
Change-Id: Ib63d1c7480cc2a5130d943e36cb0a02f9b0af7cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1890437
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711193}

chrome/browser/ui/passwords/manage_passwords_state.cc

@@ -137,22 +137,25 @@ void ManagePasswordsState::OnAutomaticPasswordSave(
 
 void ManagePasswordsState::OnPasswordAutofilled(
     const std::vector<const PasswordForm*>& password_forms,
-    const GURL& origin,
+    GURL origin,
     const std::vector<const PasswordForm*>* federated_matches) {
   DCHECK(!password_forms.empty() ||
          (federated_matches && !federated_matches->empty()));
-  ClearData();
-  local_credentials_forms_ = DeepCopyNonPSLVector(password_forms);
+  auto local_credentials_forms = DeepCopyNonPSLVector(password_forms);
   if (federated_matches)
-    AppendDeepCopyVector(*federated_matches, &local_credentials_forms_);
+    AppendDeepCopyVector(*federated_matches, &local_credentials_forms);
+
+  // Delete |form_manager_| only when the parameters are processed. They may be
+  // coming from |form_manager_|.
+  ClearData();
 
-  if (local_credentials_forms_.empty()) {
+  if (local_credentials_forms.empty()) {
     // Don't show the UI for PSL matched passwords. They are not stored for this
     // page and cannot be deleted.
-    origin_ = GURL();
-    SetState(password_manager::ui::INACTIVE_STATE);
+    OnInactive();
   } else {
-    origin_ = origin;
+    origin_ = std::move(origin);
+    local_credentials_forms_ = std::move(local_credentials_forms);
     SetState(password_manager::ui::MANAGE_STATE);
   }
 }

chrome/browser/ui/passwords/manage_passwords_state.h

@@ -70,7 +70,7 @@ class ManagePasswordsState {
   // federated credentials to show to the user as well.
   void OnPasswordAutofilled(
       const std::vector<const autofill::PasswordForm*>& password_forms,
-      const GURL& origin,
+      GURL origin,
       const std::vector<const autofill::PasswordForm*>* federated_matches);
 
   // Move to INACTIVE_STATE.

chrome/browser/ui/passwords/manage_passwords_state_unittest.cc

@@ -594,4 +594,38 @@ TEST_F(ManagePasswordsStateTest, ChooseCredentialLocalWithNonEmptyFederation) {
   passwords_data().ChooseCredential(&local_federated_form());
 }
 
+TEST_F(ManagePasswordsStateTest, AutofillCausedByInternalFormManager) {
+  struct OwningPasswordFormManagerForUI : public MockPasswordFormManagerForUI {
+    GURL origin;
+    std::vector<const autofill::PasswordForm*> best_matches;
+    std::vector<const autofill::PasswordForm*> federated_matches;
+
+    const GURL& GetOrigin() const override { return origin; }
+    const std::vector<const autofill::PasswordForm*>& GetBestMatches()
+        const override {
+      return best_matches;
+    }
+    std::vector<const autofill::PasswordForm*> GetFederatedMatches()
+        const override {
+      return federated_matches;
+    }
+  };
+
+  auto test_form_manager = std::make_unique<OwningPasswordFormManagerForUI>();
+  auto* weak_manager = test_form_manager.get();
+  test_form_manager->origin = saved_match().origin;
+  test_form_manager->best_matches = {&saved_match()};
+  test_form_manager->federated_matches = {&local_federated_form()};
+  passwords_data().OnPendingPassword(std::move(test_form_manager));
+
+  // Force autofill with the parameters coming from the object to be destroyed.
+  passwords_data().OnPasswordAutofilled(weak_manager->best_matches,
+                                        weak_manager->origin,
+                                        &weak_manager->federated_matches);
+  EXPECT_THAT(passwords_data().GetCurrentForms(),
+              UnorderedElementsAre(Pointee(local_federated_form()),
+                                   Pointee(saved_match())));
+  EXPECT_EQ(saved_match().origin, passwords_data().origin());
+}
+
 }  // namespace

components/password_manager/core/browser/http_auth_manager_impl.cc

@@ -128,6 +128,13 @@ void HttpAuthManagerImpl::OnLoginSuccesfull() {
   if (!form_manager_->is_submitted())
     return;
 
+  if (form_manager_->GetFormFetcher()->GetState() ==
+      FormFetcher::State::WAITING) {
+    // We have a provisional save manager, but it didn't finish matching yet.
+    // We just give up.
+    return;
+  }
+
   // TODO(crbug/831123) Move the logic into the PasswordFormManager.
   bool is_update = form_manager_->IsPasswordUpdate();
   bool is_new_login = form_manager_->IsNewLogin();

components/password_manager/core/browser/http_auth_manager_unittest.cc

@@ -220,4 +220,30 @@ TEST_F(HttpAuthManagerTest, NavigationWithoutSubmission) {
   httpauth_manager()->DetachObserver(&observer);
 }
 
+TEST_F(HttpAuthManagerTest, NavigationWhenMatchingNotReady) {
+  EXPECT_CALL(client_, IsSavingAndFillingEnabled).WillRepeatedly(Return(true));
+  PasswordForm observed_form;
+  observed_form.scheme = PasswordForm::Scheme::kBasic;
+  observed_form.origin = GURL("http://proxy.com/");
+  observed_form.signon_realm = "proxy.com/realm";
+
+  MockHttpAuthObserver observer;
+  // The password store is queried but it's slow and won't respond.
+  EXPECT_CALL(*store_, GetLogins);
+  // Initiate creating a form manager.
+  httpauth_manager()->SetObserverAndDeliverCredentials(&observer,
+                                                       observed_form);
+
+  PasswordForm submitted_form = observed_form;
+  submitted_form.username_value = ASCIIToUTF16("user");
+  submitted_form.password_value = ASCIIToUTF16("1234");
+  httpauth_manager()->OnPasswordFormSubmitted(submitted_form);
+  httpauth_manager()->OnPasswordFormDismissed();
+
+  // Expect no prompt as the password store didn't reply.
+  EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordPtr()).Times(0);
+  httpauth_manager()->OnDidFinishMainFrameNavigation();
+  httpauth_manager()->DetachObserver(&observer);
+}
+
 }  // namespace password_manager