New issue
Advanced search Search tips

Issue 694712 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK hit when a component policy fetch returns with empty settings_entity_id

Project Member Reported by emaxx@chromium.org, Feb 21 2017

Issue description

Chrome Version: ToT build with target_os = chromeos and dcheck_always_on = true

What steps will reproduce the problem?
(1) Set up a test policy server that responds to component policy fetches (i.e. with "google/chrome/extension" or "google/chromeos/signinextension" type) with a PolicyFetchResponse that has settings_entity_id = "".
(2) Start a Chrome instance with pointing it to the policy server (using the --device-management-url command line flag)

What is the expected result?
Chrome ignores the fetched invalid component policy.

What happens instead?
Chrome crashes:
[FATAL:resource_cache.cc(223)] Check failed: false. 
#0 0x7f7ac385d66b base::debug::StackTrace::StackTrace()
#1 0x7f7ac385bcec base::debug::StackTrace::StackTrace()
#2 0x7f7ac38c22dc logging::LogMessage::~LogMessage()
#3 0x7f7ab833933f policy::ResourceCache::VerifyKeyPathAndGetSubkeyPath()
#4 0x7f7ab8339b81 policy::ResourceCache::Delete()
#5 0x7f7ab830f0be policy::ComponentCloudPolicyStore::Delete()
#6 0x7f7ab8316f55 policy::ComponentCloudPolicyUpdater::UpdateExternalPolicy()
#7 0x7f7ab8302097 policy::ComponentCloudPolicyService::Backend::UpdateWithLastFetchedPolicy()
#8 0x7f7ab8301d0b policy::ComponentCloudPolicyService::Backend::SetCredentials()
#9 0x7f7ab830a22e _ZN4base8internal13FunctorTraitsIMN6policy27ComponentCloudPolicyService7BackendEFvRKSsS6_S6_S6_iEvE6InvokeIPS4_JS6_S6_S6_S6_RKiEEEvS8_OT_DpOT0_
#10 0x7f7ab8309ff7 _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN6policy27ComponentCloudPolicyService7BackendEFvRKSsS8_S8_S8_iEJPS6_S8_S8_S8_S8_RKiEEEvOT_DpOT0_
#11 0x7f7ab8309f28 _ZN4base8internal7InvokerINS0_9BindStateIMN6policy27ComponentCloudPolicyService7BackendEFvRKSsS7_S7_S7_iEJNS0_17UnretainedWrapperIS5_EESsSsSsSsiEEEFvvEE7RunImplI
RKS9_RKSt5tupleIJSB_SsSsSsSsiEEJLm0ELm1ELm2ELm3ELm4ELm5EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#12 0x7f7ab8309d3c _ZN4base8internal7InvokerINS0_9BindStateIMN6policy27ComponentCloudPolicyService7BackendEFvRKSsS7_S7_S7_iEJNS0_17UnretainedWrapperIS5_EESsSsSsSsiEEEFvvEE3RunEPNS0
_13BindStateBaseE
#13 0x7f7ac3862d61 _ZNO4base8internal8RunMixinINS_8CallbackIFvvELNS0_8CopyModeE0ELNS0_10RepeatModeE0EEEE3RunEv
#14 0x7f7ac3862651 base::debug::TaskAnnotator::RunTask()
#15 0x7f7ac38e92be base::MessageLoop::RunTask()
#16 0x7f7ac38e9534 base::MessageLoop::DeferOrRunPendingTask()
#17 0x7f7ac38e9824 base::MessageLoop::DoWork()
#18 0x7f7ac3900adc base::MessagePumpLibevent::Run()
#19 0x7f7ac38e8ea2 base::MessageLoop::RunHandler()
#20 0x7f7ac398c9e4 base::RunLoop::Run()
#21 0x7f7ac3a2a6e2 base::Thread::Run()
#22 0x7f7abcf4fa58 content::BrowserThreadImpl::FileThreadRun()
#23 0x7f7abcf4ff9d content::BrowserThreadImpl::Run()
#24 0x7f7ac3a2aed3 base::Thread::ThreadMain()
#25 0x7f7ac3a135fa base::(anonymous namespace)::ThreadFunc()
#26 0x7f7ac3c95184 start_thread
#27 0x7f7aac3da37d clone
 

Comment 1 by emaxx@chromium.org, Feb 21 2017

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 22 2017

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

commit ad9fcff98d11896c40edf5d0e9fca2c005b31fd2
Author: emaxx <emaxx@chromium.org>
Date: Wed Feb 22 14:58:29 2017

Fix DCHECK hit on component policy fetch with empty ID

The DCHECK was caused by the attempt to wipe the cache data with
an empty subkey, which is not allowed by policy::ResourceCache.

This CL ensures that such invalid policy gets immediately discarded
at the validation phase, without reaching the later steps like
touching cache.

BUG= 694712 
TEST=New unit tests;
     Manual test against current revision of YAPS (policy test server),
     which is currently erroneously sending such invalid component
     policy responses.

Review-Url: https://codereview.chromium.org/2706293005
Cr-Commit-Position: refs/heads/master@{#452048}

[modify] https://crrev.com/ad9fcff98d11896c40edf5d0e9fca2c005b31fd2/components/policy/core/common/cloud/component_cloud_policy_store.cc
[modify] https://crrev.com/ad9fcff98d11896c40edf5d0e9fca2c005b31fd2/components/policy/core/common/cloud/component_cloud_policy_store_unittest.cc
[modify] https://crrev.com/ad9fcff98d11896c40edf5d0e9fca2c005b31fd2/components/policy/core/common/cloud/component_cloud_policy_updater_unittest.cc

Comment 3 by emaxx@chromium.org, Feb 23 2017

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
as per #2

Sign in to add a comment