New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 711004 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
OOO until 4th Feb
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug-Regression
Team-Security-UX



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 description

UserAgent: 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
 
Cc: lgar...@chromium.org
Components: UI>Browser>Bubbles>PageInfo
lgarron@: could you take a look at this?
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?

Comment 3 by rsesek@chromium.org, 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).
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. :-(

Comment 5 by as...@dropbox.com, 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
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.

Comment 7 by f...@chromium.org, Apr 19 2017

Cc: -lgar...@chromium.org
Owner: lgar...@chromium.org
Status: Assigned (was: Unconfirmed)
Labels: Needs-Feedback
We have a similar report on Windows:  Issue 713708 .

asims@: Could you try the ideas in comment #6?
Cc: reillyg@chromium.org
Maybe related to 709852, which is a crash on android after clicking site settings.
Open chrome://settings/content, scroll down to the "USB Device" section and click "Manage". Do you see any devices listed there?

Comment 12 by as...@dropbox.com, Apr 21 2017

I do not, no.
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.
It would have to have no attributes, because otherwise it'd just send a message to nil which would be fine.

Comment 15 by akh...@dropbox.com, 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.
Cc: -reillyg@chromium.org
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
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.
Cc: lgar...@chromium.org
Components: Internals>Permissions>Model
Owner: dominickn@chromium.org
Status: Started (was: Assigned)
Summary: Page Info crashes when passed invalid permission values (which can be done with enterprise policy) (was: Clicking to access site information/permissions crashes browser)
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
Attaching a screenshot for the previous comment separately, due to Monorail glitchiness.
Screen Shot 2017-04-28 at 19.58.50.png
163 KB View Download
Cc: dominickn@chromium.org
Owner: raymes@chromium.org
Status: Assigned (was: Started)
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.
+1 we should fix this in the content settings layer. I will look into this tomorrow.

Comment 22 by as...@dropbox.com, 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.
> 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.

Comment 24 by as...@dropbox.com, May 2 2017

You appear to be correct on both counts. Thanks to all for the help.
Project Member

Comment 25 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)
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