Skip to content

Commit 695dba0

Browse files
robpavezaDevtools-frontend LUCI CQ
authored andcommitted
Simplify extension server startup
Security issues around where DevTools extensions are allowed to be run. This change mostly preserves the existing extension semantics while tightening up the rules to prevent new URL schemes from being permitted from being attached. Bug: 376303922 Change-Id: I0b25fbb2975657a682457f64848c308c54121e77 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5976474 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Robert Paveza <[email protected]>
1 parent 2280876 commit 695dba0

File tree

2 files changed

+42
-15
lines changed

2 files changed

+42
-15
lines changed

front_end/models/extensions/ExtensionServer.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,20 @@ describe('ExtensionServer', () => {
704704
assert.isTrue(Extensions.ExtensionServer.ExtensionServer.canInspectURL(url), url);
705705
}
706706
});
707+
708+
it('cannot inspect non-HTTP URL schemes', () => {
709+
const blockedUrls = [
710+
'devtools://devtools/bundled/front_end/devtools_app.html',
711+
'devtools://devtools/anything',
712+
'chrome://extensions',
713+
'chrome-untrusted://extensions',
714+
'chrome-error://crash',
715+
'chrome-search://foo/bar',
716+
];
717+
for (const url of blockedUrls as Platform.DevToolsPath.UrlString[]) {
718+
assert.isFalse(Extensions.ExtensionServer.ExtensionServer.canInspectURL(url), url);
719+
}
720+
});
707721
});
708722

709723
function assertIsStatus<T>(value: T|

front_end/models/extensions/ExtensionServer.ts

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,14 @@ import {RecorderExtensionEndpoint} from './RecorderExtensionEndpoint.js';
5656
import {RecorderPluginManager} from './RecorderPluginManager.js';
5757

5858
const extensionOrigins: WeakMap<MessagePort, Platform.DevToolsPath.UrlString> = new WeakMap();
59+
const kPermittedSchemes = ['http:', 'https:', 'file:', 'data:', 'chrome-extension:', 'about:'];
5960

6061
declare global {
6162
interface Window {
6263
DevToolsAPI?: {getInspectedTabId?(): string|undefined, getOriginsForbiddenForExtensions?(): string[]};
6364
}
6465
}
6566

66-
const kAllowedOrigins = [].map(url => (new URL(url)).origin);
67-
6867
let extensionServerInstance: ExtensionServer|null;
6968

7069
export class HostsPolicy {
@@ -1406,19 +1405,8 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
14061405
} catch (exception) {
14071406
return false;
14081407
}
1409-
if (kAllowedOrigins.includes(parsedURL.origin)) {
1410-
return true;
1411-
}
1412-
if (parsedURL.protocol === 'chrome:' || parsedURL.protocol === 'devtools:' ||
1413-
parsedURL.protocol === 'chrome-untrusted:' || parsedURL.protocol === 'chrome-error:' ||
1414-
parsedURL.protocol === 'chrome-search:') {
1415-
return false;
1416-
}
1417-
if (parsedURL.protocol.startsWith('http') && parsedURL.hostname.match(/^chrome\.google\.com\.?$/) &&
1418-
parsedURL.pathname.startsWith('/webstore')) {
1419-
return false;
1420-
}
1421-
if (parsedURL.protocol.startsWith('http') && parsedURL.hostname.match(/^chromewebstore\.google\.com\.?$/)) {
1408+
1409+
if (!kPermittedSchemes.includes(parsedURL.protocol)) {
14221410
return false;
14231411
}
14241412

@@ -1428,9 +1416,34 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
14281416
return false;
14291417
}
14301418

1419+
if (this.#isUrlFromChromeWebStore(parsedURL)) {
1420+
return false;
1421+
}
1422+
14311423
return true;
14321424
}
14331425

1426+
/**
1427+
* Tests whether a given URL is from the Chrome web store to prevent the extension server from
1428+
* being injected. This is treated as separate from the `getOriginsForbiddenForExtensions` API because
1429+
* DevTools might not be being run from a native origin and we still want to lock down this specific
1430+
* origin from DevTools extensions.
1431+
*
1432+
* @param parsedURL The URL to check
1433+
* @returns `true` if the URL corresponds to the Chrome web store; otherwise `false`
1434+
*/
1435+
static #isUrlFromChromeWebStore(parsedURL: URL): boolean {
1436+
if (parsedURL.protocol.startsWith('http') && parsedURL.hostname.match(/^chrome\.google\.com\.?$/) &&
1437+
parsedURL.pathname.startsWith('/webstore')) {
1438+
return true;
1439+
}
1440+
if (parsedURL.protocol.startsWith('http') && parsedURL.hostname.match(/^chromewebstore\.google\.com\.?$/)) {
1441+
return true;
1442+
}
1443+
1444+
return false;
1445+
}
1446+
14341447
private disableExtensions(): void {
14351448
this.extensionsEnabled = false;
14361449
}

0 commit comments

Comments
 (0)