Commit a841e109 authored by dpapad's avatar dpapad Committed by Chromium LUCI CQ

Settings: Prevent out-of-bounds access in ImportDataHandler.

The UI passes an index specifying which profile should be imported.
Previously there were no checks to ensure that the index is within
the bounds of the profiles array.

Fixed: 1164816
Change-Id: I01dd1a6d857b5af408d0b04cff429774c02368b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2619769
Auto-Submit: dpapad <dpapad@chromium.org>
Reviewed-by: default avatarRebekah Potter <rbpotter@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842222}
parent a084bdad
...@@ -40,8 +40,7 @@ const char kImportStatusSucceeded[] = "succeeded"; ...@@ -40,8 +40,7 @@ const char kImportStatusSucceeded[] = "succeeded";
const char kImportStatusFailed[] = "failed"; const char kImportStatusFailed[] = "failed";
} // namespace } // namespace
ImportDataHandler::ImportDataHandler() ImportDataHandler::ImportDataHandler() : importer_host_(nullptr) {
: importer_host_(nullptr), import_did_succeed_(false) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
} }
...@@ -115,6 +114,12 @@ void ImportDataHandler::HandleImportData(const base::ListValue* args) { ...@@ -115,6 +114,12 @@ void ImportDataHandler::HandleImportData(const base::ListValue* args) {
const base::DictionaryValue* types = nullptr; const base::DictionaryValue* types = nullptr;
CHECK(args->GetDictionary(1, &types)); CHECK(args->GetDictionary(1, &types));
if (!importer_list_loaded_ || browser_index < 0 ||
browser_index >= static_cast<int>(importer_list_->count())) {
// Prevent out-of-bounds access.
return;
}
uint16_t selected_items = importer::NONE; uint16_t selected_items = importer::NONE;
if (*types->FindBoolKey(prefs::kImportDialogAutofillFormData)) if (*types->FindBoolKey(prefs::kImportDialogAutofillFormData))
selected_items |= importer::AUTOFILL_FORM_DATA; selected_items |= importer::AUTOFILL_FORM_DATA;
...@@ -180,6 +185,7 @@ void ImportDataHandler::HandleImportFromBookmarksFile( ...@@ -180,6 +185,7 @@ void ImportDataHandler::HandleImportFromBookmarksFile(
void ImportDataHandler::SendBrowserProfileData(const std::string& callback_id) { void ImportDataHandler::SendBrowserProfileData(const std::string& callback_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
importer_list_loaded_ = true;
base::ListValue browser_profiles; base::ListValue browser_profiles;
for (size_t i = 0; i < importer_list_->count(); ++i) { for (size_t i = 0; i < importer_list_->count(); ++i) {
......
...@@ -69,7 +69,8 @@ class ImportDataHandler : public SettingsPageUIHandler, ...@@ -69,7 +69,8 @@ class ImportDataHandler : public SettingsPageUIHandler,
// of deleting itself when import is complete. // of deleting itself when import is complete.
ExternalProcessImporterHost* importer_host_; // weak ExternalProcessImporterHost* importer_host_; // weak
bool import_did_succeed_; bool import_did_succeed_{false};
bool importer_list_loaded_{false};
scoped_refptr<ui::SelectFileDialog> select_file_dialog_; scoped_refptr<ui::SelectFileDialog> select_file_dialog_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment