Security: debugger extension API is too powerful |
||||||||||||||||||||||||
Issue descriptionMy understanding of Chrome's security model regarding extensions is as follows: Users can grant almost complete access to data that is stored inside a browser profile to extensions. Authorization for accessing such data is given through the prompt that lists extension permissions at install time. If the user is signed in on multiple devices and sync is active, an extension that is installed on one device will also silently be pushed to the other devices (unless prevented by an enterprise policy). This is supposed to be fine even if the devices are trusted to different degrees, or have different owners, because the pushed extension can only operate on the synced browser profile, not the rest of the machine. But if an extension can use the debugger permission to affect system state beyond the scope of the browser profile, this security model is violated. One function in the debugger API that clearly goes beyond the bounds of the current browser profile is "Page.setDownloadBehavior" (https://chromedevtools.github.io/devtools-protocol/tot/Page#method-setDownloadBehavior). Quoting the documentation: > Set the behavior when downloading a file. > PARAMETERS > behavior: Whether to allow all or deny all download requests, or use default Chrome behavior if available (otherwise deny).deny, allow, default > downloadPath: The default path to save downloaded files to. This is requred if behavior is set to 'allow' I have tested that this works; the following code, executed in the context of an extension with debugger permission, will drop a file called "authorized_keys" in /home/user/.ssh/ (unless such a file already exists); nonexistent directories will be created. chrome.tabs.create({url:'about:blank'},({id: child_id}) => { let target = {tabId: child_id}; chrome.debugger.attach(target, '1.3', ()=>{ chrome.debugger.sendCommand(target, 'Page.setDownloadBehavior', {behavior:'allow', downloadPath:'/home/user/.ssh'}, (res) => { chrome.debugger.sendCommand(target, 'Runtime.evaluate', {expression: ` var blob_url = URL.createObjectURL(new Blob(['hello world'], {type:'application/octet-stream'})); document.body.innerHTML='<a download="authorized_keys" id=x href="'+blob_url+'">foo</a>'; x.click(); `}, (res) => { console.log('evaluate: ', res); }); }); }); }); Interestingly, this even works on Chrome OS; you can drop files in locations like /home/chronos or /media, even though normally not even the user can directly access /home/chronos (as far as I can tell). You can also use the debugger API to put a breakpoint in extensions::binding and grab references to local variables (like the "require" and "requireNative" functions), and synchronously call them to bypass the check for disabled natives. However, I'm not sure whether you can actually do anything particularly bad from that context. I'm attaching my demo code. The tip-of-tree API documentation also lists "Page.addCompilationCache" (https://chromedevtools.github.io/devtools-protocol/tot/Page#method-addCompilationCache), but that doesn't seem to be available in Chrome yet; it looks like this was only added a couple days ago (https://chromium.googlesource.com/chromium/src/+/155210a0f5fc6f6871ad1ecc1ce04333411ebd7e)? As far as I can tell, https://v8project.blogspot.com/2015/07/code-caching.html describes the same mechanism as used by this API; in the comments section (https://v8project.blogspot.com/2015/07/code-caching.html?showComment=1438070292453#c2899136651658395474), a V8 developer explains that this API is not supposed to be exposed to untrusted data. I have tested that an extension with debugger permission can be propagated from one device to another via sync. Given that the debugger protocol seems to be developed under the assumption that it won't be exposed to untrusted code, I think a reasonable way to fix this would be to require stronger user consent, e.g. a commandline flag and a butterbar (similar to flags like --no-sandbox), before permitting the use of extensions with debugger permission. This bug is subject to a 90 day disclosure deadline. After 90 days elapse or a patch has been made broadly available (whichever is earlier), the bug report will become visible to the public.
,
Jul 23
Setting component and some labels. This would affect all OS with extensions. Tentatively setting this as Severity-High since on it's own it seems like this could be exploitable on ChromeOS (at least), but it also seems that at least the file-write trick might be usable as part of a sandbox escape (and at minimum appears to violate the intent of permissions on ChromeOS). cc'ing rdevlin.cronin and meacer for perspective on extensions/permissions.
,
Jul 23
We do show an infobar "<Extension name> is debugging this browser", which should stop any debugging sessions if closed.
,
Jul 23
Thanks for the report! This is interesting, but I don't know if (most of it) qualifies as a security bug. I think there's a few assumptions about the security model of extensions that aren't quite accurate. Comments below. > This is supposed to be fine even if the devices are trusted to different degrees, or have different owners, because the pushed extension can only operate on the synced browser profile, not the rest of the machine. There is no such guarantee in the extensions security model. With the proper permissions, extensions can affect contents outside of the browser profile. A simple example is the downloads API, which allows the extension to download files to the user's machine. Similarly, I don't think the debugger API assumes that it can only operate within the bounds of a single profile and not have any lasting effect outside of the browser profile. > Interestingly, this even works on Chrome OS; you can drop files in locations like /home/chronos or /media, even though normally not even the user can directly access /home/chronos (as far as I can tell). This does seem like a bug. > You can also use the debugger API to put a breakpoint in extensions::binding and grab references to local variables (like the "require" and "requireNative" functions), and synchronously call them to bypass the check for disabled natives. However, I'm not sure whether you can actually do anything particularly bad from that context. I'm attaching my demo code. There's relatively little you can do here *assuming you already have the debugger permission*. It allows you to e.g. fake a user gesture, but you can already do that with the debugger permission. Similarly for other renderer actions. Calling any privileged extension API function will result in a message sent to the browser process, and the browser process will perform its own permission checks. > Given that the debugger protocol seems to be developed under the assumption that it won't be exposed to untrusted code I don't know that I'd say this is correct. We have a higher bar for what we allow to use the debugger permission, but I wouldn't say it has to be completely trusted code. Instead, we assume that any consumer of the debugger API has been installed intentionally by the user, with proper disclosure. > I think a reasonable way to fix this would be to require stronger user consent, e.g. a commandline flag and a butterbar (similar to flags like --no-sandbox), before permitting the use of extensions with debugger permission. Don't we do this? I thought we have a persistent infobar when the debugger is attached to a web contents, unless the extension is a policy-installed extension or the scary "silent-debugger-extension-api" switch is used. [1] [1] https://chromium.googlesource.com/chromium/src/+/b5b7d1a13bda0aace4dc1b83d07ce66d5a2be25c/chrome/browser/extensions/api/debugger/debugger_api.cc#220 ------------ Overall, I think most of this falls into the "WAI" bucket. It's absolutely true that the debugger API is big and scary, and can do some very nasty things. However, that's the reason we have fairly persistent and scary warnings for users. I think there's probably more we can do here, too (e.g., we've talked about requiring the user to be in developer mode in order to allow any extension to use the debugger API), but I don't think that makes this a security vulnerability. meacer@, dgozman@, I'm curious for your thoughts as well. Note that one part that does stand out to me is the ability to write to disk in the /home/chronos directory - that definitely seems like a bug, and I'm not sure what badness could be caused by it.
,
Jul 23
> There is no such guarantee in the extensions security model. With the proper permissions, extensions can affect contents outside of the browser profile. A simple example is the downloads API, which allows the extension to download files to the user's machine.
So you're saying that if a user signs their personal gmail account in to Chrome both on a personal machine and a work machine (with sync enabled), and an attacker then manages to compromise the personal machine, it is WAI that the attacker can compromise the work machine unless the work machine explicitly blacklists all scary permissions?
> Instead, we assume that any consumer of the debugger API has been installed intentionally by the user, with proper disclosure.
That's only true if none of the user's machines have been compromised. I believe it is an important distinction whether you have consent from the user on another machine or from the user on the machine running the code.
> Don't we do this? I thought we have a persistent infobar when the debugger is attached to a web contents, unless the extension is a policy-installed extension or the scary "silent-debugger-extension-api" switch is used. [1]
Yeah, there's an infobar, but it's only visible as long as the debugger is attached. If I run some of the requests synchronously, I can't even see the infobar flash up:
chrome.tabs.create({url:'about:blank'},({id: child_id}) => {
let target = {tabId: child_id};
chrome.debugger.attach(target, '1.3', ()=>{});
chrome.debugger.sendCommand(target, 'Page.setDownloadBehavior', {behavior:'allow', downloadPath:'/home/user/.ssh'}, (res) => {});
chrome.debugger.sendCommand(target, 'Runtime.evaluate', {expression: `
var blob_url = URL.createObjectURL(new Blob(['hello world 123'], {type:'application/octet-stream'}));
document.body.innerHTML='<a download="authorized_keys" id=x href="'+blob_url+'">foo</a>';
x.click();
`}, (res) => {chrome.debugger.detach(target, ()=>{})});
});
If I run them all asynchronously as intended, the debugger is attached a bit longer, and I can see that some bar is very briefly flashing up, but can't make out the text.
,
Jul 23
> So you're saying that if a user signs their personal gmail account in to Chrome both on a personal machine and a work machine (with sync enabled), and an attacker then manages to compromise the personal machine, it is WAI that the attacker can compromise the work machine unless the work machine explicitly blacklists all scary permissions? Depends on your perspective. If the user has a malicious extension installed on their personal account, then having sync add that extension to the user's account on other machines they are signed into is proper behavior. Theoretically, a malicious extension should not be able to compromise a machine, but it does depend on what you consider compromised. (e.g., we could say "leaking user information" is compromised, and then any sync'd extension with the tabs permission can "compromise" any number of machines the user is signed into.) > That's only true if none of the user's machines have been compromised. I believe it is an important distinction whether you have consent from the user on another machine or from the user on the machine running the code. I agree with this, but it applies to sync as a whole. There are definitely risks associated with using any kind of remote consent. We've thought about ways that we can help mitigate these risks (e.g., having a measure of "trustworthiness" for different clients, and prompting the user if a sync node comes from an untrusted client). However, this bug is about the debugger API, not about syncing extensions, and I'd prefer to keep them separate. > Yeah, there's an infobar, but it's only visible as long as the debugger is attached. If I run some of the requests synchronously, I can't even see the infobar flash up: This is a reasonable point, and it's one of the reasons we've been looking at what other requirements (such as a user in developer mode) we can add. We could, I suppose, also add a "minimum visible time" to the infobar, but I'm not sure that's a good idea.
,
Jul 23
> Theoretically, a malicious extension should not be able to compromise a machine, but it does depend on what you consider compromised. (e.g., we could say "leaking user information" is compromised, and then any sync'd extension with the tabs permission can "compromise" any number of machines the user is signed into.) This is what I meant when I wrote "affect system state beyond the scope of the browser profile" - I believe that access to anything happening in the browser profile (e.g. cookies, bookmarks and tab contents in the profile) is WAI, but anything that affects system state beyond that (overriding download restrictions, opening downloaded files, sandboxed native code execution) is not.
,
Jul 23
We don't have a distinction between "on one machine" and "on any machine the extension is installed on" in terms of the security model, and as I mentioned above, there's also not the assumption that the extension can only ever affect state within the given browser profile. While it would in many ways be nice to reign in some of these capabilities, it doesn't mean that it is currently not WAI or should be classified as a security vulnerability.
,
Jul 25
,
Jul 30
rdevlin.cronin: Mind owning this while we decide what to do here? Feel free to pass it to someone else if you think they'd be more appropriate, or back to me for re-triage if you aren't sure. meacer, dgozman: Friendly ping from the security sheriff. See c#4.
,
Jul 30
This API came up back when 805445 was filed, and rob.wu pointed that it's primarily intended for headless Chrome. If that's the case, can we at least disable it for non-headless Chrome? Link to Rob's comment: https://bugs.chromium.org/p/chromium/issues/detail?id=805445#c10
,
Jul 31
I have no objection to making the setDownloadBehavior method restricted to headless chrome, but I don't have a lot of context into it. I'll defer to dgozman@ there. Since dgozman@ also owns devtools, passing ownership of the bug. However, I still think we need to take a longer look at this bug and decide a) if this is a security concern at all and b) if it is, to what extent we want to change behavior. While setDownloadBehavior() is a single way to affect content outside the profile, there's quite a few others (e.g. downloads) that I don't think are inherently high-risk. meacer@ (or other security folks): is the larger behavior described here (extensions modifying state outside the immediate user profile) WAI? If so, I'd say we close this out and file a targeted bug for any specific enhancements we wanted to do (e.g., restricting setDownloadBehavior() to headless, having debugger extensions require dev mode, etc).
,
Aug 7
dgozman@ seems to be OOO till Aug 20. Are we fine to wait or would it make sense to assign another owner for now?
,
Aug 7
Given there's some debate over whether this is a bug or WAI, I'd say we're fine to wait - but just my $0.02.
,
Aug 7
dgozman: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 21
dgozman: Uh oh! This issue still open and hasn't been updated in the last 28 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 5
,
Sep 21
We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue? For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 3
The 90-day deadline for public disclosure of this bug is coming up in 18 days.
,
Oct 5
,
Oct 5
I think this is working as intended in general. That said, let me craft a CL which explicitly addresses setDownloadBehavior.
,
Oct 5
,
Oct 5
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6 commit c71d8045ce0592cf3f4290744ab57b23c1d1b4c6 Author: Dmitry Gozman <dgozman@chromium.org> Date: Tue Oct 09 18:40:45 2018 [DevTools] Do not allow Page.setDownloadBehavior for extensions Bug: 866426 Change-Id: I71b672978e1a8ec779ede49da16b21198567d3a4 Reviewed-on: https://chromium-review.googlesource.com/c/1270007 Commit-Queue: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#598004} [modify] https://crrev.com/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6/chrome/browser/extensions/api/debugger/debugger_api.cc [modify] https://crrev.com/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6/chrome/test/data/extensions/api_test/debugger/background.js [modify] https://crrev.com/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6/content/browser/devtools/protocol/page_handler.cc [modify] https://crrev.com/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6/content/browser/devtools/protocol/page_handler.h [modify] https://crrev.com/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6/content/browser/devtools/render_frame_devtools_agent_host.cc [modify] https://crrev.com/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6/content/public/browser/devtools_agent_host_client.cc [modify] https://crrev.com/c71d8045ce0592cf3f4290744ab57b23c1d1b4c6/content/public/browser/devtools_agent_host_client.h
,
Oct 11
Closing this now, with setDownloadBehavior addressed. Most of the debugger protocol is working as intended, so if you find any similar issues, new bugs are welcome!
,
Oct 12
To clarify: Does this mean that an extension with debugger privilege that can write to the host filesystem is considered a security bug; but an extension that can corrupt V8-internal state and probably gain code execution in a sandboxed renderer using Page.addCompilationCache is working as intended?
,
Oct 12
,
Oct 15
,
Oct 15
I assume that this fix will not land in stable within the next 20 days, and I'll therefore derestrict the entry in our bugtracker in a week. If you do plan to fix this in stable within the next 20 days, please let me know.
,
Oct 26
,
Oct 26
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 26
(Already in 71)
,
Dec 3
,
Dec 11
,
Dec 11
,
Jan 18
(4 days ago)
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||
Comment 1 by yangguo@chromium.org
, Jul 23