Skip to content

Commit

Permalink
Passwords LoginDatabase: Fix primary key returned from AddLogin
Browse files Browse the repository at this point in the history
AddLogin calls `db_.GetLastInsertRowId()` to determine the primary key
of the newly-added credential. However, in the "replace" code path, it
called GetLastInsertRowId() twice, and the second time, it might
return an incorrect result, since other DB insertions (insecure
credentials and/or password notes) might have happened in the
meantime.
This CL fixes the issue by not calling GetLastInsertRowId() a second
time, and instead reusing the value from the first call.

(cherry picked from commit 1d969eb)

Bug: 1352460
Change-Id: I50319678fa160959c9f766e71f3b53b0398c284a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3841758
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1037059}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3838572
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Auto-Submit: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/branch-heads/5249@{#36}
Cr-Branched-From: 4f7bea5-refs/heads/main@{#1036826}
  • Loading branch information
Marc Treib authored and Chromium LUCI CQ committed Aug 23, 2022
1 parent 4a33041 commit f679fae
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions components/password_manager/core/browser/login_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1076,8 +1076,7 @@ PasswordStoreChangeList LoginDatabase::AddLogin(const PasswordForm& form,
}
UpdatePasswordNotes(primary_key, form_with_encrypted_password.notes);
list.emplace_back(PasswordStoreChange::ADD,
std::move(form_with_encrypted_password),
FormPrimaryKey(db_.GetLastInsertRowId()),
std::move(form_with_encrypted_password), primary_key,
password_changed, insecure_changed);
} else if (error) {
if (db_error_handler.get_error_code() == 19 /*SQLITE_CONSTRAINT*/) {
Expand Down

0 comments on commit f679fae

Please sign in to comment.