Skip to content

Commit cdcca1a

Browse files
pfaffeDevtools-frontend LUCI CQ
authored andcommitted
Check file access on the host side of the extension api
Fixed: 406034851 Change-Id: I125bfa572ba9e987569e8524da99de84db83d389 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6401296 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Philip Pfaffe <[email protected]>
1 parent 4091180 commit cdcca1a

File tree

3 files changed

+59
-74
lines changed

3 files changed

+59
-74
lines changed

front_end/models/extensions/ExtensionAPI.ts

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -699,9 +699,7 @@ self.injectedExtensionAPI = function(
699699
userAction = true;
700700
try {
701701
const {resource, lineNumber} = message as {resource: APIImpl.ResourceData, lineNumber: number};
702-
if (canAccessResource(resource)) {
703-
callback.call(null, new (Constructor(Resource))(resource), lineNumber);
704-
}
702+
callback.call(null, new (Constructor(Resource))(resource), lineNumber);
705703
} finally {
706704
userAction = false;
707705
}
@@ -1228,39 +1226,17 @@ self.injectedExtensionAPI = function(
12281226
},
12291227
};
12301228

1231-
const protocolGet = Object.getOwnPropertyDescriptor(URL.prototype, 'protocol')?.get;
1232-
function getProtocol(url: string): string {
1233-
if (!protocolGet) {
1234-
throw new Error('URL.protocol is not available');
1235-
}
1236-
return protocolGet.call(new URL(url));
1237-
}
1238-
1239-
function canAccessResource(resource: APIImpl.ResourceData): boolean {
1240-
try {
1241-
return extensionInfo.allowFileAccess || getProtocol(resource.url) !== 'file:';
1242-
} catch {
1243-
return false;
1244-
}
1245-
}
1246-
12471229
function InspectedWindow(this: PublicAPI.Chrome.DevTools.InspectedWindow): void {
12481230
function dispatchResourceEvent(
12491231
this: APIImpl.EventSink<(resource: APIImpl.Resource) => unknown>, message: {arguments: unknown[]}): void {
12501232
const resourceData = message.arguments[0] as APIImpl.ResourceData;
1251-
if (!canAccessResource(resourceData)) {
1252-
return;
1253-
}
12541233
this._fire(new (Constructor(Resource))(resourceData));
12551234
}
12561235

12571236
function dispatchResourceContentEvent(
12581237
this: APIImpl.EventSink<(resource: APIImpl.Resource, content: string) => unknown>,
12591238
message: {arguments: unknown[]}): void {
12601239
const resourceData = message.arguments[0] as APIImpl.ResourceData;
1261-
if (!canAccessResource(resourceData)) {
1262-
return;
1263-
}
12641240
this._fire(new (Constructor(Resource))(resourceData), message.arguments[1] as string);
12651241
}
12661242

@@ -1323,16 +1299,13 @@ self.injectedExtensionAPI = function(
13231299
return new (Constructor(Resource))(resourceData);
13241300
}
13251301
function callbackWrapper(resources: unknown): void {
1326-
callback?.((resources as APIImpl.ResourceData[]).filter(canAccessResource).map(wrapResource));
1302+
callback?.((resources as APIImpl.ResourceData[]).map(wrapResource));
13271303
}
13281304
extensionServer.sendRequest({command: PrivateAPI.Commands.GetPageResources}, callback && callbackWrapper);
13291305
},
13301306
};
13311307

13321308
function ResourceImpl(this: APIImpl.Resource, resourceData: APIImpl.ResourceData): void {
1333-
if (!canAccessResource(resourceData)) {
1334-
throw new Error('Resource access not allowed');
1335-
}
13361309
this._url = resourceData.url;
13371310
this._type = resourceData.type;
13381311
}

front_end/models/extensions/ExtensionServer.test.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,13 @@ describeWithDevtoolsExtension('Runtime hosts policy', {hostsPolicy}, context =>
618618
assert.exists(context.chrome.devtools);
619619
const resources =
620620
await new Promise<Chrome.DevTools.Resource[]>(r => context.chrome.devtools?.inspectedWindow.getResources(r));
621-
assert.deepEqual(resources.map(r => r.url), [blockedUrl, allowedUrl]);
621+
assert.deepEqual(resources.map(r => r.url), [allowedUrl]);
622622

623623
const resourceContents = await Promise.all(resources.map(
624624
resource => new Promise<{url: string, content?: string, encoding?: string}>(
625625
r => resource.getContent((content, encoding) => r({url: resource.url, content, encoding})))));
626626

627627
assert.deepEqual(resourceContents, [
628-
{url: blockedUrl, content: undefined, encoding: undefined},
629628
{url: allowedUrl, content: 'content', encoding: ''},
630629
]);
631630
});
@@ -683,16 +682,16 @@ describeWithDevtoolsExtension('Runtime hosts policy', {hostsPolicy}, context =>
683682
assert.exists(context.chrome.devtools);
684683
const resources =
685684
await new Promise<Chrome.DevTools.Resource[]>(r => context.chrome.devtools?.inspectedWindow.getResources(r));
686-
assert.deepEqual(resources.map(r => r.url), [blockedUrl, allowedUrl]);
685+
assert.deepEqual(resources.map(r => r.url), [allowedUrl]);
687686

688687
assert.deepEqual(project.uiSourceCodeForURL(allowedUrl)?.content(), 'content');
689688
assert.deepEqual(project.uiSourceCodeForURL(blockedUrl)?.content(), 'content');
690689
const responses = await Promise.all(resources.map(
691690
resource => new Promise<Object|undefined>(r => resource.setContent('modified', true, r)))) as
692691
Array<undefined|{code: string, details: string[]}>;
693692

694-
assert.deepEqual(responses.map(response => response?.code), ['E_FAILED', 'OK']);
695-
assert.deepEqual(responses.map(response => response?.details), [['Permission denied'], []]);
693+
assert.deepEqual(responses.map(response => response?.code), ['OK']);
694+
assert.deepEqual(responses.map(response => response?.details), [[]]);
696695

697696
assert.deepEqual(project.uiSourceCodeForURL(allowedUrl)?.content(), 'modified');
698697
assert.deepEqual(project.uiSourceCodeForURL(blockedUrl)?.content(), 'content');

front_end/models/extensions/ExtensionServer.ts

Lines changed: 53 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -281,27 +281,27 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
281281
}
282282

283283
notifySearchAction(panelId: string, action: string, searchString?: string): void {
284-
this.postNotification(PrivateAPI.Events.PanelSearch + panelId, action, searchString);
284+
this.postNotification(PrivateAPI.Events.PanelSearch + panelId, [action, searchString]);
285285
}
286286

287287
notifyViewShown(identifier: string, frameIndex?: number): void {
288-
this.postNotification(PrivateAPI.Events.ViewShown + identifier, frameIndex);
288+
this.postNotification(PrivateAPI.Events.ViewShown + identifier, [frameIndex]);
289289
}
290290

291291
notifyViewHidden(identifier: string): void {
292-
this.postNotification(PrivateAPI.Events.ViewHidden + identifier);
292+
this.postNotification(PrivateAPI.Events.ViewHidden + identifier, []);
293293
}
294294

295295
notifyButtonClicked(identifier: string): void {
296-
this.postNotification(PrivateAPI.Events.ButtonClicked + identifier);
296+
this.postNotification(PrivateAPI.Events.ButtonClicked + identifier, []);
297297
}
298298

299299
profilingStarted(): void {
300-
this.postNotification(PrivateAPI.Events.ProfilingStarted);
300+
this.postNotification(PrivateAPI.Events.ProfilingStarted, []);
301301
}
302302

303303
profilingStopped(): void {
304-
this.postNotification(PrivateAPI.Events.ProfilingStopped);
304+
this.postNotification(PrivateAPI.Events.ProfilingStopped, []);
305305
}
306306

307307
private registerLanguageExtensionEndpoint(
@@ -449,14 +449,17 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
449449
return this.status.OK();
450450
}
451451

452-
private onSetFunctionRangesForScript(message: PrivateAPI.ExtensionServerRequestMessage): Record {
452+
private onSetFunctionRangesForScript(message: PrivateAPI.ExtensionServerRequestMessage, port: MessagePort): Record {
453453
if (message.command !== PrivateAPI.Commands.SetFunctionRangesForScript) {
454454
return this.status.E_BADARG('command', `expected ${PrivateAPI.Commands.SetFunctionRangesForScript}`);
455455
}
456456
const {scriptUrl, ranges} = message;
457457
if (!scriptUrl || !ranges?.length) {
458458
return this.status.E_BADARG('command', 'expected valid scriptUrl and non-empty NamedFunctionRanges');
459459
}
460+
if (!this.extensionAllowedOnURL(scriptUrl as Platform.DevToolsPath.UrlString, port)) {
461+
return this.status.E_FAILED('Permission denied');
462+
}
460463
const uiSourceCode =
461464
Workspace.Workspace.WorkspaceImpl.instance().uiSourceCodeForURL(scriptUrl as Platform.DevToolsPath.UrlString);
462465
if (!uiSourceCode) {
@@ -527,7 +530,7 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
527530
this.requests = new Map();
528531
this.enableExtensions();
529532
const url = event.data.inspectedURL();
530-
this.postNotification(PrivateAPI.Events.InspectedURLChanged, url);
533+
this.postNotification(PrivateAPI.Events.InspectedURLChanged, [url]);
531534
const extensions = this.#pendingExtensions.splice(0);
532535
extensions.forEach(e => this.addExtension(e));
533536
}
@@ -536,32 +539,27 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
536539
return this.subscribers.has(type);
537540
}
538541

539-
private isNotificationAllowedForExtension(port: MessagePort, type: string, ..._args: unknown[]): boolean {
540-
if (type === PrivateAPI.Events.NetworkRequestFinished) {
541-
const entry = _args[1] as HAR.Log.EntryDTO;
542-
const origin = extensionOrigins.get(port);
543-
const extension = origin && this.registeredExtensions.get(origin);
544-
if (extension?.isAllowedOnTarget(entry.request.url)) {
545-
return true;
546-
}
547-
return false;
548-
}
549-
return true;
550-
}
551-
552-
private postNotification(type: string, ..._vararg: unknown[]): void {
542+
private postNotification(type: string, args: unknown[], filter?: (extension: RegisteredExtension) => boolean): void {
553543
if (!this.extensionsEnabled) {
554544
return;
555545
}
556546
const subscribers = this.subscribers.get(type);
557547
if (!subscribers) {
558548
return;
559549
}
560-
const message = {command: 'notify-' + type, arguments: Array.prototype.slice.call(arguments, 1)};
550+
const message = {command: 'notify-' + type, arguments: args};
561551
for (const subscriber of subscribers) {
562-
if (this.extensionEnabled(subscriber) && this.isNotificationAllowedForExtension(subscriber, type, ..._vararg)) {
563-
subscriber.postMessage(message);
552+
if (!this.extensionEnabled(subscriber)) {
553+
continue;
564554
}
555+
if (filter) {
556+
const origin = extensionOrigins.get(subscriber);
557+
const extension = origin && this.registeredExtensions.get(origin);
558+
if (!extension || !filter(extension)) {
559+
continue;
560+
}
561+
}
562+
subscriber.postMessage(message);
565563
}
566564
}
567565

@@ -848,8 +846,10 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
848846

849847
private handleOpenURL(
850848
port: MessagePort, contentProvider: TextUtils.ContentProvider.ContentProvider, lineNumber: number): void {
851-
port.postMessage(
852-
{command: 'open-resource', resource: this.makeResource(contentProvider), lineNumber: lineNumber + 1});
849+
if (this.extensionAllowedOnURL(contentProvider.contentURL(), port)) {
850+
port.postMessage(
851+
{command: 'open-resource', resource: this.makeResource(contentProvider), lineNumber: lineNumber + 1});
852+
}
853853
}
854854

855855
private extensionAllowedOnURL(url: Platform.DevToolsPath.UrlString, port: MessagePort): boolean {
@@ -936,7 +936,8 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
936936
}>();
937937
function pushResourceData(
938938
this: ExtensionServer, contentProvider: TextUtils.ContentProvider.ContentProvider): boolean {
939-
if (!resources.has(contentProvider.contentURL())) {
939+
if (!resources.has(contentProvider.contentURL()) &&
940+
this.extensionAllowedOnURL(contentProvider.contentURL(), port)) {
940941
resources.set(contentProvider.contentURL(), this.makeResource(contentProvider));
941942
}
942943
return false;
@@ -999,7 +1000,8 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
9991000
return undefined;
10001001
}
10011002

1002-
private onAttachSourceMapToResource(message: PrivateAPI.ExtensionServerRequestMessage): Record|undefined {
1003+
private onAttachSourceMapToResource(message: PrivateAPI.ExtensionServerRequestMessage, port: MessagePort): Record
1004+
|undefined {
10031005
if (message.command !== PrivateAPI.Commands.AttachSourceMapToResource) {
10041006
return this.status.E_BADARG('command', `expected ${PrivateAPI.Commands.GetResourceContent}`);
10051007
}
@@ -1009,6 +1011,9 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
10091011
}
10101012

10111013
const url = message.contentUrl as Platform.DevToolsPath.UrlString;
1014+
if (!this.extensionAllowedOnURL(url, port)) {
1015+
return this.status.E_FAILED('Permission denied');
1016+
}
10121017
const contentProvider = Workspace.Workspace.WorkspaceImpl.instance().uiSourceCodeForURL(url);
10131018
if (!contentProvider) {
10141019
return this.status.E_NOTFOUND(url);
@@ -1145,34 +1150,42 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper<EventTyp
11451150

11461151
private notifyResourceAdded(event: Common.EventTarget.EventTargetEvent<Workspace.UISourceCode.UISourceCode>): void {
11471152
const uiSourceCode = event.data;
1148-
this.postNotification(PrivateAPI.Events.ResourceAdded, this.makeResource(uiSourceCode));
1153+
this.postNotification(
1154+
PrivateAPI.Events.ResourceAdded, [this.makeResource(uiSourceCode)],
1155+
extension => extension.isAllowedOnTarget(uiSourceCode.url()));
11491156
}
11501157

11511158
private notifyUISourceCodeContentCommitted(
11521159
event: Common.EventTarget.EventTargetEvent<Workspace.Workspace.WorkingCopyCommitedEvent>): void {
11531160
const {uiSourceCode, content} = event.data;
1154-
this.postNotification(PrivateAPI.Events.ResourceContentCommitted, this.makeResource(uiSourceCode), content);
1161+
this.postNotification(
1162+
PrivateAPI.Events.ResourceContentCommitted, [this.makeResource(uiSourceCode), content],
1163+
extension => extension.isAllowedOnTarget(uiSourceCode.url()));
11551164
}
11561165

11571166
private async notifyRequestFinished(event: Common.EventTarget.EventTargetEvent<SDK.NetworkRequest.NetworkRequest>):
11581167
Promise<void> {
11591168
const request = event.data;
11601169
const entry = await HAR.Log.Entry.build(request, {sanitize: false});
1161-
this.postNotification(PrivateAPI.Events.NetworkRequestFinished, this.requestId(request), entry);
1170+
this.postNotification(
1171+
PrivateAPI.Events.NetworkRequestFinished, [this.requestId(request), entry],
1172+
extension => extension.isAllowedOnTarget(entry.request.url));
11621173
}
11631174

11641175
private notifyElementsSelectionChanged(): void {
1165-
this.postNotification(PrivateAPI.Events.PanelObjectSelected + 'elements');
1176+
this.postNotification(PrivateAPI.Events.PanelObjectSelected + 'elements', []);
11661177
}
11671178

11681179
sourceSelectionChanged(url: Platform.DevToolsPath.UrlString, range: TextUtils.TextRange.TextRange): void {
1169-
this.postNotification(PrivateAPI.Events.PanelObjectSelected + 'sources', {
1170-
startLine: range.startLine,
1171-
startColumn: range.startColumn,
1172-
endLine: range.endLine,
1173-
endColumn: range.endColumn,
1174-
url,
1175-
});
1180+
this.postNotification(
1181+
PrivateAPI.Events.PanelObjectSelected + 'sources', [{
1182+
startLine: range.startLine,
1183+
startColumn: range.startColumn,
1184+
endLine: range.endLine,
1185+
endColumn: range.endColumn,
1186+
url,
1187+
}],
1188+
extension => extension.isAllowedOnTarget(url));
11761189
}
11771190

11781191
private setInspectedTabId(event: Common.EventTarget.EventTargetEvent<string>): void {

0 commit comments

Comments
 (0)