PVer4 code should filter out the safe extensions when calling OnCheckExtensionsResult |
|||||||
Issue description
PVer3 code used to filter out safe extensions from the call to OnCheckExtensionsResult but PVer4 code does not:
PVer3:
case EXTENSIONBLACKLIST: {
std::set<std::string> unsafe_extension_ids;
for (size_t i = 0; i < full_hashes.size(); ++i) {
std::string extension_id = SBFullHashToString(full_hashes[i]);
if (full_hash_results[i] == SB_THREAT_TYPE_EXTENSION)
unsafe_extension_ids.insert(extension_id);
}
client->OnCheckExtensionsResult(unsafe_extension_ids);
break;
}
PVer4:
case ClientCallbackType::CHECK_EXTENSION_IDS: {
const std::set<FullHash> extension_ids(check->full_hashes.begin(),
check->full_hashes.end());
check->client->OnCheckExtensionsResult(extension_ids);
break;
Fix: Filter out the extension IDs that are safe.
See: http://b/36085194
,
Mar 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ec975c1d7990d87c1be9c03e0d94e38e20a7389b commit ec975c1d7990d87c1be9c03e0d94e38e20a7389b Author: vakh <vakh@chromium.org> Date: Fri Mar 10 23:15:24 2017 PVer4: Filter out the safe crxs before calling OnCheckExtensionsResult The argument for OnCheckExtensionsResult should only contain the bad extensions, but PVer4 code was including all CRXs (blacklisted or not). BUG= 700145 Review-Url: https://codereview.chromium.org/2742913002 Cr-Commit-Position: refs/heads/master@{#456212} [modify] https://crrev.com/ec975c1d7990d87c1be9c03e0d94e38e20a7389b/components/safe_browsing_db/v4_local_database_manager.cc [modify] https://crrev.com/ec975c1d7990d87c1be9c03e0d94e38e20a7389b/components/safe_browsing_db/v4_local_database_manager.h [modify] https://crrev.com/ec975c1d7990d87c1be9c03e0d94e38e20a7389b/components/safe_browsing_db/v4_local_database_manager_unittest.cc
,
Mar 13 2017
Requesting to allow merge of the fix of this bug to 57 for the following reasons: - Causes all extensions in user's browser to be uploaded to Google. Only the unsafe extensions should have been uploaded. - Trying to upload all extensions also resulted in DOS'ing a critical Google server used for SafeBrowsing. - The feature is Finch controlled so it can be rolled back easily in case there are any issues.
,
Mar 13 2017
This bug requires manual review: We are only 0 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 13 2017
Withdrawing merge request after talking with govind@ since the bar for merge is "safe and critical" patches.
,
Mar 13 2017
"Commit ec975c1d7990d87c1be9c03e0d94e38e20a7389b initially landed in 59.0.3038.0"
,
Mar 13 2017
Requesting to allow merge of the fix of this bug to 58 for the following reasons: - Causes all extensions in user's browser to be uploaded to Google. Only the unsafe extensions should have been uploaded. - Trying to upload all extensions also resulted in DOS'ing a critical Google server used for SafeBrowsing. - The feature is Finch controlled so it can be rolled back easily in case there are any issues.
,
Mar 13 2017
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), bhthompson@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ad44e53304cef79c5ca790aab3fe7a23e674be7a commit ad44e53304cef79c5ca790aab3fe7a23e674be7a Author: Jialiu Lin <jialiul@chromium.org> Date: Mon Mar 13 21:20:45 2017 PVer4: Filter out the safe crxs before calling OnCheckExtensionsResult The argument for OnCheckExtensionsResult should only contain the bad extensions, but PVer4 code was including all CRXs (blacklisted or not). BUG= 700145 Review-Url: https://codereview.chromium.org/2742913002 Cr-Commit-Position: refs/heads/master@{#456212} (cherry picked from commit ec975c1d7990d87c1be9c03e0d94e38e20a7389b) Review-Url: https://codereview.chromium.org/2747053002 . Cr-Commit-Position: refs/branch-heads/3029@{#164} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/ad44e53304cef79c5ca790aab3fe7a23e674be7a/components/safe_browsing_db/v4_local_database_manager.cc [modify] https://crrev.com/ad44e53304cef79c5ca790aab3fe7a23e674be7a/components/safe_browsing_db/v4_local_database_manager.h [modify] https://crrev.com/ad44e53304cef79c5ca790aab3fe7a23e674be7a/components/safe_browsing_db/v4_local_database_manager_unittest.cc |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by vakh@chromium.org
, Mar 10 2017