New issue
Advanced search Search tips

Issue 866462 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Use RegKey's with less access in reporter_runner_win.cc

Project Member Reported by joenotcharles@chromium.org, Jul 23

Issue description

chrome/browser/safe_browsing/chrome_cleaner/reporter_runner_win.cc creates RegKey's with KEY_ALL_ACCESS. The common pattern is to read the value of the key, report it through UMA, and then delete the value. They only need KEY_QUERY_VALUE and KEY_SET_VALUE access for his.

Also, the constructor used will create the key if it doesn't already exist. To avoid that, construct an empty RegKey and then use Open. First, though, check to make sure we don't depend on this behaviour (for instance maybe we rely on the key existing with a default value).
 
Labels: SafeBrowsing-Triaged
Labels: Hotlist-CodeHealth
Labels: Hotlist-GoodFirstBug
Owner: mguaypaq@chromium.org
It looks like this was half done at some point, but there are a few uses of KEY_ALL_ACCESS left to fix.

Also there are some in chromium/src/chrome/browser/component_updater/sw_reporter_installer_win.cc and various files under chrome/chrome_cleaner.
Each of these files is covered by a different unit test:

reporter_runner_win.cc's test are in browser_tests (not really unit tests; more like integration tests for Chrome)
sw_reporter_installer_win.cc's tests are in unit_tests (the main Chrome browser's unit tests)
The rest are in chrome_cleaner_unittests


Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 8

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

commit 987de34818251d9545ea3591df022d7a7dc44cd2
Author: Mathieu Guay-Paquet <mguaypaq@chromium.org>
Date: Wed Aug 08 16:38:15 2018

Make ReportUMAForLastCleanerRun public so it can be tested

Bug: 866462
Change-Id: If09ba70f7c777482fd1225b08047231876c3938b
Reviewed-on: https://chromium-review.googlesource.com/1156854
Commit-Queue: Mathieu Guay-Paquet <mguaypaq@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581593}
[modify] https://crrev.com/987de34818251d9545ea3591df022d7a7dc44cd2/chrome/browser/component_updater/sw_reporter_installer_win.cc
[modify] https://crrev.com/987de34818251d9545ea3591df022d7a7dc44cd2/chrome/browser/component_updater/sw_reporter_installer_win.h

Sign in to add a comment