Skip to content

Commit 7848024

Browse files
szuendDevtools-frontend LUCI CQ
authored andcommitted
[extensions] Allow arbitrary protocol in 'sourceURL' magic comments
Currently, the extension API disallows interacting with resources from certain protocol (e.g. 'chrome://'). The check works by allow-listing only a narrow set of protocols, such as 'https://' and 'http://'. The problem is that this check also prevents access to JavaScript scripts, that are valid to access. E.g. webpack bundles that use 'eval' + 'sourceURL' results in scripts with a 'webpack://' prefix. Or react server rendering results in scripts with 'reactsr://'. This CL softens the check slightly by allowing extensions to list and modify resources that stem from 'sourceURL' magic comments provided the inspected target is itself allowed to be inspected. As DevTools unifies UISourceCode's under the hood, this also means that ALL targets that provide the same script resource need to be inspectable. [email protected] Fixed: 416196401 Change-Id: I68616cde318bcb986b4999fafb8a3b04d90dfd02 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6822154 Reviewed-by: Philip Pfaffe <[email protected]> Commit-Queue: Simon Zünd <[email protected]>
1 parent b0a0347 commit 7848024

File tree

2 files changed

+104
-33
lines changed

2 files changed

+104
-33
lines changed

front_end/models/extensions/ExtensionServer.test.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -687,15 +687,14 @@ describeWithDevtoolsExtension('Runtime hosts policy', {hostsPolicy}, context =>
687687
});
688688

689689
async function createUISourceCode(
690-
project: Bindings.ContentProviderBasedProject.ContentProviderBasedProject, url: Platform.DevToolsPath.UrlString) {
690+
project: Bindings.ContentProviderBasedProject.ContentProviderBasedProject, url: Platform.DevToolsPath.UrlString,
691+
contentType = Common.ResourceType.resourceTypes.Document) {
691692
const mimeType = 'text/html';
692693
const dataProvider = () =>
693694
Promise.resolve(new TextUtils.ContentData.ContentData('content', /* isBase64 */ false, mimeType));
694695
project.addUISourceCodeWithProvider(
695-
new Workspace.UISourceCode.UISourceCode(project, url, Common.ResourceType.resourceTypes.Document),
696-
new TextUtils.StaticContentProvider.StaticContentProvider(
697-
url, Common.ResourceType.resourceTypes.Document, dataProvider),
698-
null, mimeType);
696+
new Workspace.UISourceCode.UISourceCode(project, url, contentType),
697+
new TextUtils.StaticContentProvider.StaticContentProvider(url, contentType, dataProvider), null, mimeType);
699698
await project.uiSourceCodeForURL(url)?.requestContentData();
700699
}
701700

@@ -726,6 +725,32 @@ describeWithDevtoolsExtension('Runtime hosts policy', {hostsPolicy}, context =>
726725
]);
727726
});
728727

728+
it('allows arbitrary schemes in sourceURL comments, as long as the inspected target is allowed', async () => {
729+
const target = createTarget({id: 'target' as Protocol.Target.TargetID});
730+
target.setInspectedURL(allowedUrl);
731+
732+
const script = sinon.createStubInstance(SDK.Script.Script, {target, contentURL: blockedUrl});
733+
script.hasSourceURL = true;
734+
const workspaceBinding = sinon.createStubInstance(Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding);
735+
workspaceBinding.scriptsForUISourceCode.callsFake(uiSourceCode => {
736+
if (uiSourceCode.contentURL() === blockedUrl) {
737+
return [script];
738+
}
739+
return [];
740+
});
741+
sinon.stub(Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding, 'instance').returns(workspaceBinding);
742+
const project = new Bindings.ContentProviderBasedProject.ContentProviderBasedProject(
743+
Workspace.Workspace.WorkspaceImpl.instance(), target.id(), Workspace.Workspace.projectTypes.Network, '',
744+
false /* isServiceProject */);
745+
await createUISourceCode(project, blockedUrl, Common.ResourceType.resourceTypes.Script);
746+
await createUISourceCode(project, allowedUrl, Common.ResourceType.resourceTypes.Script);
747+
748+
assert.exists(context.chrome.devtools);
749+
const resources =
750+
await new Promise<Chrome.DevTools.Resource[]>(r => context.chrome.devtools?.inspectedWindow.getResources(r));
751+
assert.deepEqual(resources.map(r => r.url), [blockedUrl, allowedUrl]);
752+
});
753+
729754
function createRequest(
730755
networkManager: SDK.NetworkManager.NetworkManager, frameId: Protocol.Page.FrameId,
731756
requestId: Protocol.Network.RequestId, url: Platform.DevToolsPath.UrlString): void {

front_end/models/extensions/ExtensionServer.ts

Lines changed: 74 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -463,14 +463,11 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
463463
if (!scriptUrl || !ranges?.length) {
464464
return this.status.E_BADARG('command', 'expected valid scriptUrl and non-empty NamedFunctionRanges');
465465
}
466-
if (!this.extensionAllowedOnURL(scriptUrl as Platform.DevToolsPath.UrlString, port)) {
467-
return this.status.E_FAILED('Permission denied');
468-
}
469-
const uiSourceCode =
470-
Workspace.Workspace.WorkspaceImpl.instance().uiSourceCodeForURL(scriptUrl as Platform.DevToolsPath.UrlString);
471-
if (!uiSourceCode) {
472-
return this.status.E_NOTFOUND(scriptUrl);
466+
const resource = this.lookupAllowedUISourceCode(scriptUrl as Platform.DevToolsPath.UrlString, port);
467+
if ('error' in resource) {
468+
return resource.error;
473469
}
470+
const {uiSourceCode} = resource;
474471
if (!uiSourceCode.contentType().isScript() || !uiSourceCode.contentType().isFromSourceMap()) {
475472
return this.status.E_BADARG('command', `expected a source map script resource for url: ${scriptUrl}`);
476473
}
@@ -865,17 +862,18 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
865862
private handleOpenURL(
866863
port: MessagePort, contentProviderOrUrl: TextUtils.ContentProvider.ContentProvider|string, lineNumber?: number,
867864
columnNumber?: number): void {
868-
let url: Platform.DevToolsPath.UrlString;
869865
let resource: {url: string, type: string};
866+
let isAllowed: boolean;
870867
if (typeof contentProviderOrUrl !== 'string') {
871-
url = contentProviderOrUrl.contentURL();
872868
resource = this.makeResource(contentProviderOrUrl);
869+
isAllowed = this.extensionAllowedOnContentProvider(contentProviderOrUrl, port);
873870
} else {
874-
url = contentProviderOrUrl as Platform.DevToolsPath.UrlString;
871+
const url = contentProviderOrUrl as Platform.DevToolsPath.UrlString;
875872
resource = {url, type: Common.ResourceType.resourceTypes.Other.name()};
873+
isAllowed = this.extensionAllowedOnURL(url, port);
876874
}
877875

878-
if (this.extensionAllowedOnURL(url, port)) {
876+
if (isAllowed) {
879877
port.postMessage({
880878
command: 'open-resource',
881879
resource,
@@ -891,6 +889,58 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
891889
return Boolean(extension?.isAllowedOnTarget(url));
892890
}
893891

892+
/**
893+
* Slightly more permissive as {@link extensionAllowedOnURL}: This method also permits
894+
* UISourceCodes that originate from a {@link SDK.Script.Script} with a sourceURL magic comment as
895+
* long as the corresponding target is permitted.
896+
*/
897+
private extensionAllowedOnContentProvider(
898+
contentProvider: TextUtils.ContentProvider.ContentProvider, port: MessagePort): boolean {
899+
if (!(contentProvider instanceof Workspace.UISourceCode.UISourceCode)) {
900+
return this.extensionAllowedOnURL(contentProvider.contentURL(), port);
901+
}
902+
903+
if (contentProvider.contentType() !== Common.ResourceType.resourceTypes.Script) {
904+
// We only check sourceURL magic comments for scripts (excluding ones coming from source maps).
905+
return this.extensionAllowedOnURL(contentProvider.contentURL(), port);
906+
}
907+
908+
const scripts =
909+
Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance().scriptsForUISourceCode(contentProvider);
910+
if (scripts.length === 0) {
911+
return this.extensionAllowedOnURL(contentProvider.contentURL(), port);
912+
}
913+
914+
return scripts.every(script => {
915+
if (script.hasSourceURL) {
916+
return this.extensionAllowedOnTarget(script.target(), port);
917+
}
918+
return this.extensionAllowedOnURL(script.contentURL(), port);
919+
});
920+
}
921+
922+
/**
923+
* This method prefers returning 'Permission denied' errors if restricted resources are not found,
924+
* rather then NOTFOUND. This prevents extensions from being able to fish for restricted resources.
925+
*/
926+
private lookupAllowedUISourceCode(url: Platform.DevToolsPath.UrlString, port: MessagePort):
927+
{uiSourceCode: Workspace.UISourceCode.UISourceCode}|{
928+
error: Record,
929+
}
930+
{
931+
const uiSourceCode = Workspace.Workspace.WorkspaceImpl.instance().uiSourceCodeForURL(url);
932+
if (!uiSourceCode && !this.extensionAllowedOnURL(url, port)) {
933+
return {error: this.status.E_FAILED('Permission denied')};
934+
}
935+
if (!uiSourceCode) {
936+
return {error: this.status.E_NOTFOUND(url)};
937+
}
938+
if (!this.extensionAllowedOnContentProvider(uiSourceCode, port)) {
939+
return {error: this.status.E_FAILED('Permission denied')};
940+
}
941+
return {uiSourceCode};
942+
}
943+
894944
private extensionAllowedOnTarget(target: SDK.Target.Target, port: MessagePort): boolean {
895945
return this.extensionAllowedOnURL(target.inspectedURL(), port);
896946
}
@@ -980,7 +1030,7 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
9801030
function pushResourceData(
9811031
this: ExtensionServer, contentProvider: TextUtils.ContentProvider.ContentProvider): boolean {
9821032
if (!resources.has(contentProvider.contentURL()) &&
983-
this.extensionAllowedOnURL(contentProvider.contentURL(), port)) {
1033+
this.extensionAllowedOnContentProvider(contentProvider, port)) {
9841034
resources.set(contentProvider.contentURL(), this.makeResource(contentProvider));
9851035
}
9861036
return false;
@@ -1003,7 +1053,7 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
10031053
private async getResourceContent(
10041054
contentProvider: TextUtils.ContentProvider.ContentProvider, message: PrivateAPI.ExtensionServerRequestMessage,
10051055
port: MessagePort): Promise<void> {
1006-
if (!this.extensionAllowedOnURL(contentProvider.contentURL(), port)) {
1056+
if (!this.extensionAllowedOnContentProvider(contentProvider, port)) {
10071057
this.dispatchCallback(message.requestId, port, this.status.E_FAILED('Permission denied'));
10081058
return undefined;
10091059
}
@@ -1053,20 +1103,16 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
10531103
return this.status.E_FAILED('Expected a source map URL but got null');
10541104
}
10551105

1056-
const url = message.contentUrl as Platform.DevToolsPath.UrlString;
1057-
if (!this.extensionAllowedOnURL(url, port)) {
1058-
return this.status.E_FAILED('Permission denied');
1059-
}
1060-
const contentProvider = Workspace.Workspace.WorkspaceImpl.instance().uiSourceCodeForURL(url);
1061-
if (!contentProvider) {
1062-
return this.status.E_NOTFOUND(url);
1106+
const resource = this.lookupAllowedUISourceCode(message.contentUrl as Platform.DevToolsPath.UrlString, port);
1107+
if ('error' in resource) {
1108+
return resource.error;
10631109
}
10641110

10651111
const debuggerBindingsInstance = Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance();
1066-
const scriptFiles = debuggerBindingsInstance.scriptsForUISourceCode(contentProvider);
1112+
const scriptFiles = debuggerBindingsInstance.scriptsForUISourceCode(resource.uiSourceCode);
10671113
if (scriptFiles.length > 0) {
10681114
for (const script of scriptFiles) {
1069-
const resourceFile = debuggerBindingsInstance.scriptFile(contentProvider, script.debuggerModel);
1115+
const resourceFile = debuggerBindingsInstance.scriptFile(resource.uiSourceCode, script.debuggerModel);
10701116
resourceFile?.addSourceMapURL(message.sourceMapURL as Platform.DevToolsPath.UrlString);
10711117
}
10721118
}
@@ -1084,13 +1130,13 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
10841130
const response = error ? this.status.E_FAILED(error) : this.status.OK();
10851131
this.dispatchCallback(requestId, port, response);
10861132
}
1087-
if (!this.extensionAllowedOnURL(url as Platform.DevToolsPath.UrlString, port)) {
1088-
return this.status.E_FAILED('Permission denied');
1089-
}
10901133

1091-
const uiSourceCode =
1092-
Workspace.Workspace.WorkspaceImpl.instance().uiSourceCodeForURL(url as Platform.DevToolsPath.UrlString);
1093-
if (!uiSourceCode?.contentType().isDocumentOrScriptOrStyleSheet()) {
1134+
const resource = this.lookupAllowedUISourceCode(url as Platform.DevToolsPath.UrlString, port);
1135+
if ('error' in resource) {
1136+
return resource.error;
1137+
}
1138+
const {uiSourceCode} = resource;
1139+
if (!uiSourceCode.contentType().isDocumentOrScriptOrStyleSheet()) {
10941140
const resource = SDK.ResourceTreeModel.ResourceTreeModel.resourceForURL(url as Platform.DevToolsPath.UrlString);
10951141
if (!resource) {
10961142
return this.status.E_NOTFOUND(url);

0 commit comments

Comments
 (0)