Issue metadata
Sign in to add a comment
|
Page Info crashes when passed invalid permission values (which can be done with enterprise policy)
Reported by
as...@dropbox.com,
Apr 12 2017
|
||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36 Steps to reproduce the problem: 1. go to any site (http or https, but not chrome://) 2. click left side of URL bar for site information 3. instant crash What is the expected behavior? see a list of site information and elect permissions What went wrong? ¯\_(ツ)_/¯ Crashed report ID: Crash ID bdbd95ac-2b55-437c-94f6-25128da99ea0 (Server ID: fe0cfcbe10000000) How much crashed? Whole browser Is it a problem with a plugin? No Did this work before? Yes before last update Chrome version: 57.0.2987.133 Channel: stable OS Version: OS X 10.12.4 Flash Version: 24.0.0.221
,
Apr 13 2017
I can't reproduce on that particular OS + Chrome version. Some possible leads: 1. Does this happen every time for every site? 2. Does it happen if you use a different profile? 3. Does it happen on Beta/Canary? 4. Are you using Web Bluetooth or Web USB? 5. Are you using --secondary-ui-md, or any other flags that might affect UI or permissions?
,
Apr 13 2017
List Annotations Crashing on exception: NSMutableRLEArray objectAtIndex:effectiveRange:: Out of bounds Thread 0 CRASHED [EXC_BAD_INSTRUCTION / EXC_I386_INVOP @ 0x00007fff96f1f792 ] MAGIC SIGNATURE THREAD 0x00007fff96f1f792 (AppKit + 0x003a5792 ) -[NSApplication _crashOnException:] 0x00007fff96f1f6c5 (AppKit + 0x003a56c5 ) -[NSApplication reportException:] 0x00007fff96ff573e (AppKit + 0x0047b73e ) uncaughtErrorProc 0x00007fff991ccff9 (CoreFoundation + 0x0018fff9 ) __handleUncaughtException 0x00007fffadf446f4 (libobjc.A.dylib + 0x000176f4 ) _objc_terminate() 0x00007fffad433d68 (libc++abi.dylib + 0x00023d68 ) std::__terminate(void (*)()) 0x00007fffad4337dd (libc++abi.dylib + 0x000237dd ) __cxa_throw 0x00007fffadf425b5 (libobjc.A.dylib + 0x000155b5 ) objc_exception_throw 0x00007fff991ccc3c (CoreFoundation + 0x0018fc3c ) +[NSException raise:format:] 0x00007fff9ab00f72 (Foundation + 0x0004cf72 ) -[NSRLEArray objectAtIndex:effectiveRange:] 0x0000000111113637 (Google Chrome Framework -website_settings_utils_cocoa.mm:16 ) SizeForWebsiteSettingsButtonTitle(NSPopUpButton*, NSString*) 0x000000011110d01f (Google Chrome Framework -permission_selector_button.mm:44 ) -[PermissionSelectorButton initWithPermissionInfo:forURL:withCallback:profile:] 0x0000000111110f42 (Google Chrome Framework -website_settings_bubble_controller.mm:739 ) -[WebsiteSettingsBubbleController addPopUpButtonForPermission:toView:atPoint:] 0x000000011111188a (Google Chrome Framework -website_settings_bubble_controller.mm:858 ) -[WebsiteSettingsBubbleController addPermission:toView:atPoint:] 0x0000000111112c65 (Google Chrome Framework -website_settings_bubble_controller.mm:1109 ) -[WebsiteSettingsBubbleController setPermissionInfo:andChosenObjects:] 0x000000011111346d (Google Chrome Framework -website_settings_bubble_controller.mm:1216 ) WebsiteSettingsUIBridge::SetPermissionInfo(std::__1::vector<WebsiteSettingsUI::PermissionInfo, std::__1::allocator<WebsiteSettingsUI::PermissionInfo> > const&, std::__1::vector<std::__1::unique_ptr<WebsiteSettingsUI::ChosenObjectInfo, std::__1::default_delete<WebsiteSettingsUI::ChosenObjectInfo> >, std::__1::allocator<std::__1::unique_ptr<WebsiteSettingsUI::ChosenObjectInfo, std::__1::default_delete<WebsiteSettingsUI::ChosenObjectInfo> > > >) 0x0000000110e6d725 (Google Chrome Framework -website_settings.cc:692 ) WebsiteSettings::PresentSitePermissions() 0x0000000110e6bafd (Google Chrome Framework -website_settings.cc:234 ) WebsiteSettings::WebsiteSettings(WebsiteSettingsUI*, Profile*, TabSpecificContentSettings*, content::WebContents*, GURL const&, security_state::SecurityInfo const&) 0x00000001111132d0 (Google Chrome Framework -website_settings_bubble_controller.mm:1186 ) WebsiteSettingsUIBridge::Show(NSWindow*, Profile*, content::WebContents*, GURL const&, security_state::SecurityInfo const&) 0x0000000110eba31c (Google Chrome Framework -browser_commands.cc:912 ) chrome::ShowWebsiteSettings(Browser*, content::WebContents*) 0x00000001110ac3d4 (Google Chrome Framework -location_icon_decoration.mm:126 ) LocationIconDecoration::OnMousePressed(CGRect, CGPoint) 0x00000001110ade44 (Google Chrome Framework -security_state_bubble_decoration.mm:268 ) SecurityStateBubbleDecoration::OnMousePressed(CGRect, CGPoint) 0x00000001110a1be9 (Google Chrome Framework -autocomplete_text_field_cell.mm:551 ) -[AutocompleteTextFieldCell mouseDown:inRect:ofView:] 0x000000011109ed2d (Google Chrome Framework -autocomplete_text_field.mm:176 ) -[AutocompleteTextField mouseDown:] 0x00007fff974ba2be (AppKit + 0x009402be ) -[NSWindow(NSEventRouting) _handleMouseDownEvent:isDelayedEvent:] 0x00007fff974b6adb (AppKit + 0x0093cadb ) -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] 0x00007fff974b5f79 (AppKit + 0x0093bf79 ) -[NSWindow(NSEventRouting) sendEvent:] 0x000000011105338e (Google Chrome Framework -chrome_event_processing_window.mm:72 ) -[ChromeEventProcessingWindow sendEvent:] 0x00007fff9733a6f0 (AppKit + 0x007c06f0 ) -[NSApplication(NSEvent) sendEvent:] 0x000000010e571ddb (Google Chrome Framework -chrome_browser_application_mac.mm:277 ) __34-[BrowserCrApplication sendEvent:]_block_invoke 0x000000010e9bdf59 (Google Chrome Framework + 0x019b6f59 ) base::mac::CallWithEHFrame(void () block_pointer) 0x000000010e571bb4 (Google Chrome Framework -chrome_browser_application_mac.mm:261 ) -[BrowserCrApplication sendEvent:] 0x00007fff96bb57f6 (AppKit + 0x0003b7f6 ) -[NSApplication run] 0x000000010e9cdb1d (Google Chrome Framework -message_pump_mac.mm:637 ) base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) 0x000000010e9cd19b (Google Chrome Framework -message_pump_mac.mm:210 ) base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) 0x000000010e9eb802 (Google Chrome Framework -run_loop.cc:37 ) base::RunLoop::Run() 0x000000010e576f48 (Google Chrome Framework -chrome_browser_main.cc:1977 ) ChromeBrowserMainParts::MainMessageLoopRun(int*) 0x000000010dd45033 (Google Chrome Framework -browser_main_loop.cc:1182 ) content::BrowserMainLoop::RunMainMessageLoopParts() 0x000000010dd47f41 (Google Chrome Framework -browser_main_runner.cc:141 ) content::BrowserMainRunnerImpl::Run() 0x000000010dd40b7b (Google Chrome Framework -browser_main.cc:46 ) content::BrowserMain(content::MainFunctionParams const&) 0x000000010e52e6ef (Google Chrome Framework -content_main_runner.cc:793 ) content::ContentMainRunnerImpl::Run() 0x000000010e52d9b5 (Google Chrome Framework -content_main.cc:20 ) content::ContentMain(content::ContentMainParams const&) 0x000000010d00a5ba (Google Chrome Framework -chrome_main.cc:112 ) ChromeMain 0x000000010c696d99 (Google Chrome -chrome_exe_main_mac.c:85 ) main 0x00007fffae827234 (libdyld.dylib + 0x00005234 ) start 0x00007fffae827234 (libdyld.dylib + 0x00005234 ) start The line crashing is: [[button attributedTitle] attributesAtIndex:0 effectiveRange:NULL]; So there is no attributeAtIndex:0 (array length is 0).
,
Apr 13 2017
Thanks for the stack trace! I also found the crash report, and have been digging into it. I still don't have a good idea under what conditions that would happen, though. :-(
,
Apr 13 2017
Answer inline 1. Does this happen every time for every site? > Yes 2. Does it happen if you use a different profile? > Different profile and incog 3. Does it happen on Beta/Canary? > Not sure to what this question refers 4. Are you using Web Bluetooth or Web USB? > USB 5. Are you using --secondary-ui-md, or any other flags that might affect UI or permissions? > secondary-ui-md is not enabled, but the following are active and may have something to do... #disable-hosted-app-shim-creation, #disable-javascript-harmony-shipping, #disable-hyperlink-auditing
,
Apr 13 2017
Enabling Web USB doesn't crash for me, hmm. Beta and Canary are upcoming Chrome versions: https://www.chromium.org/getting-involved/dev-channel Could you try them to see if they work? If Canary crashes, then it's a more urgent problem. Else, this might already be fixed. Could you try also two more things in your regular install? - Disable #enable-webusb and #enable-experimental-web-platform-features - Browser through all the esceptions at chrome://settings/content and see if there's anything unexpected.
,
Apr 19 2017
,
Apr 21 2017
We have a similar report on Windows: Issue 713708 . asims@: Could you try the ideas in comment #6?
,
Apr 21 2017
,
Apr 21 2017
Maybe related to 709852, which is a crash on android after clicking site settings.
,
Apr 21 2017
Open chrome://settings/content, scroll down to the "USB Device" section and click "Manage". Do you see any devices listed there?
,
Apr 21 2017
I do not, no.
,
Apr 21 2017
This is not related to WebUSB. The crash occurs in SizeForWebsiteSettingsButtonTitle (recently renamed to SizeForPageInfoButtonTitle) which is called for regular permissions, not the kind of chooser permissions used for WebUSB: https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/page_info/page_info_utils_cocoa.mm I'm guessing that [button attributedTitle] is either nil or has no attributes.
,
Apr 24 2017
It would have to have no attributes, because otherwise it'd just send a message to nil which would be fine.
,
Apr 26 2017
We believe the bug is in a typo in our policies. https://www.chromium.org/administrators/policy-list-3#DefaultPluginsSetting ``` If this policy is left not set, 'AllowPlugins' will be used and the user will be able to change it. 1 = Allow all sites to automatically run plugins 2 = Block all plugins 3 = Click to play ``` 4 = Crash Setting it back to 3 seems to fix it. Still worth fixing in chrome, IMHO, to handle this gracefully.
,
Apr 26 2017
Re #15: Thanks for the update! How are you able to set the value to 4? I can only set the value to 1, 2, or 3 using the admin interface, and the policy specification only allows those values: https://cs.chromium.org/chromium/src/components/policy/resources/policy_templates.json?q=%22%27name%27:+%27DefaultPluginsSetting%27,%22&l=3095 There is an internal content setting of DETECT for Flash/Plugins, but it has a value of 5, not 4. Also, the instructions for debugging a policy locally [2] don't seem to work on Sierra (and the crash has only been reported on Mac), and I can't figure out how to hack the admin tool to accept malformed data. Still working on it. [2] https://www.chromium.org/administrators/mac-quick-start
,
Apr 26 2017
Okay, I was able to trigger the crash by hacking the mandatory plist. I'll figure out what Chrome should do here, although it would still be interesting to know how the value could accidentally be set to 4.
,
Apr 29 2017
Here is my best characterization of this bug:
If there is an invalid enterprise setting (like a value of 4, meaning SESSION_ONLY, for plugins), then HostContentSettingsMap::GetWebsiteSetting() happily returns that value.
This goes through a whole bunch of (hacky, but not relevant to this bug) processing into PageInfoUI::PermissionActionToUIString() [1].
An invalid value gets mapped to kInvalidResourceID. There is a DCHECK:
DCHECK_NE(button_text_id, kInvalidResourceID);
... but even if you take it out, the next line crashes on a local build when trying to fetch the string for an invalid resource ID on the next line.
A simple fix would be to copy what is done PageInfoUI::PermissionDecisionReasonToUIString() [2] and change the DCHECK to:
if (button_text_id == kInvalidResourceID)
return base::string16();
However, that doesn't work, because then then SizeForPageInfoButtonTitle() crashes when trying to trying to get attributes at character 0 of an empty string. That's the stack trace seen above.
We can hack around that, but then Page Info will display an empty uneditable dropdown (see screenshot).
The root cause is that we are using and storing invalid permission values in the first place. And then we are asking the UI to explain invalid values to the user.
We can fix this in the UI, but it's sketchy. For example, the two places I mentioned above are not enough. PageInfoUI::PermissionActionToUIString() and PageInfoUI::PermissionDecisionReasonToUIString() assume that the setting value they are passed is safe to use as an index into e.g. kPermissionButtonTextIDPolicyManaged. A value of 4 maps to kInvalidResourceID, but a value greater than 5 will be out of bounds, and we don't check for that either.
And even if we just show *something* to prevent the crash, should we display bogus fallback UI for what we know to be invalid values?
That is all to say, I think the existing DCHECK in PageInfoUI::PermissionActionToUIString() is the right thing, and we should modify a lower layer to make sure that it doesn't pass a value that will fail the DCHECK.
Dominick, could you decide whether it is reasonable to expect that HostContentSettingsMap() should always return such a valid value? (Or pass it on to someone else?)
asims@, akhawe@: I'm still very interested in how you set a value of 4, since the admin web UI doesn't support that.
(In particular, I'd know to know how likely it is that someone else might have the same issue.)
[1] https://cs.chromium.org/chromium/src/components/content_settings/core/browser/host_content_settings_map.cc?type=cs&q=%22HostContentSettingsMap::GetWebsiteSetting%22&sq=package:chromium&l=777
[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/page_info/page_info_ui.cc?type=cs&q=%22PageInfoUI::PermissionActionToUIString%22&l=267
[3] https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/page_info/page_info_utils_cocoa.mm?q=SizeForPageInfoButtonTitle&dr=C&l=13
,
Apr 29 2017
Attaching a screenshot for the previous comment separately, due to Monorail glitchiness.
,
May 1 2017
Thanks for the report. lgarron: I think I agree with you that HostContentSettingsMap::GetWebsiteSetting should avoid return invalid values. If 1, 2, and 3 are the only valid enterprise settings, then anything else should be rejected at that level (probably falling back to DEFAULT in that case). Over to +raymes for thoughts.
,
May 1 2017
+1 we should fix this in the content settings layer. I will look into this tomorrow.
,
May 2 2017
I am unsure (1) how to change the value for DefaultPluginsSetting and (2) whether the value for that setting is in fact '4' on my machine, and I am unsure where to check it. That said, it is entirely possible this was changed to an invalid value enterprise wide. If you could give me the steps necessary to confirm that incorrect value, I will fly this up the flagpole and see what I can get from our people internally.
,
May 2 2017
> I am unsure (1) how to change the value for DefaultPluginsSetting and (2) whether the value for that setting is in fact '4' on my machine, and I am unsure where to check it. https://www.chromium.org/administrators/mac-quick-start lists the locations, if you're curious, and chrome://policy should list the value. But I hear it's fixed for Dropbox corp-manage profiles now. > I will fly this up the flagpole and see what I can get from our people internally. I believe Dev has access to the enterprise policy.
,
May 2 2017
You appear to be correct on both counts. Thanks to all for the help.
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/26e1b6d4e1d7e27840055e3091520a43f59dde41 commit 26e1b6d4e1d7e27840055e3091520a43f59dde41 Author: raymes <raymes@chromium.org> Date: Wed May 10 06:17:53 2017 Ensure settings returned from Content Settings providers are valid This CL adds DCHECKs to the providers we control to ensure that the settings returned are valid. It also adds conditional logic to ensure the policy provider never returns invalid settings. In the case of the policy provider the settings come from prefs which can be set by enterprise admins. BUG= 711004 TBR=rdevlin.cronin@chromium.org Review-Url: https://codereview.chromium.org/2853983002 Cr-Commit-Position: refs/heads/master@{#470498} [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/chrome/browser/content_settings/content_settings_internal_extension_provider.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/chrome/browser/content_settings/content_settings_policy_provider_unittest.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/chrome/browser/content_settings/host_content_settings_map_unittest.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/chrome/browser/extensions/api/content_settings/content_settings_api.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/content_settings_default_provider.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/content_settings_info.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/content_settings_info.h [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/content_settings_policy_provider.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/content_settings_pref.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/content_settings_registry.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/content_settings_registry_unittest.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/host_content_settings_map.cc [modify] https://crrev.com/26e1b6d4e1d7e27840055e3091520a43f59dde41/components/content_settings/core/browser/host_content_settings_map.h
,
May 14 2017
,
Jun 21 2017
commit 110620d3ae89e65a86ab1afd6149f6f6aa3c6d0c Author: raymes <raymes@chromium.org> Date: Mon May 15 18:36:29 2017 -0700 Fix a crash in Content Settings PolicyProvider::UpdateManagedDefaultSetting This crash was introduced in https://chromium.googlesource.com/chromium/src/+/26e1b6d4e1d7e27840055e3091520a43f59dde41 where we began checking if managed default settings were valid before loading them from the policy. However, not all content settings that can have managed defaults are registered on every platform. So we need to check that the setting is registered before checking if it is valid. BUG=721769 Review-Url: https://codereview.chromium.org/2882883002 Cr-Commit-Position: refs/heads/master@{#471974} https://bugs.chromium.org/p/chromium/issues/detail?id=721769 You do not have permission to view the requested page. Reason: User is not allowed to view this issue |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by dullweber@chromium.org
, Apr 13 2017Components: UI>Browser>Bubbles>PageInfo