From b04e784d9ec23f61aef801f3071c8fc5cffc2b8d Mon Sep 17 00:00:00 2001 From: Philip Pfaffe Date: Fri, 8 Nov 2024 17:56:47 +0000 Subject: [PATCH] Do not pass file urls to language plugins Unless the owning extension has filesystem access enabled. Bug: none Change-Id: I3d71f635974cc348d3741fd8335cdf63fbc7fec0 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5999725 Commit-Queue: Philip Pfaffe Reviewed-by: Danil Somsikov --- front_end/models/extensions/BUILD.gn | 1 + .../models/extensions/ExtensionServer.test.ts | 85 +++++++++++++++++++ .../models/extensions/ExtensionServer.ts | 8 +- .../LanguageExtensionEndpoint.test.ts | 59 +++++++++++++ .../extensions/LanguageExtensionEndpoint.ts | 31 +++++-- front_end/models/extensions/extensions.ts | 4 + .../debugger-language-plugins_test.ts | 45 ++++++---- test/e2e/helpers/extension-helpers.ts | 4 +- 8 files changed, 212 insertions(+), 25 deletions(-) create mode 100644 front_end/models/extensions/LanguageExtensionEndpoint.test.ts diff --git a/front_end/models/extensions/BUILD.gn b/front_end/models/extensions/BUILD.gn index 3eabeec926..914d668d49 100644 --- a/front_end/models/extensions/BUILD.gn +++ b/front_end/models/extensions/BUILD.gn @@ -64,6 +64,7 @@ ts_library("unittests") { sources = [ "ExtensionServer.test.ts", "HostUrlPattern.test.ts", + "LanguageExtensionEndpoint.test.ts", "RecorderPluginManager.test.ts", ] diff --git a/front_end/models/extensions/ExtensionServer.test.ts b/front_end/models/extensions/ExtensionServer.test.ts index 93b1c429b3..167ab2bb41 100644 --- a/front_end/models/extensions/ExtensionServer.test.ts +++ b/front_end/models/extensions/ExtensionServer.test.ts @@ -787,6 +787,47 @@ describeWithDevtoolsExtension('Wasm extension API', {}, context => { }); }); +class StubLanguageExtension implements Chrome.DevTools.LanguageExtensionPlugin { + async addRawModule(): Promise { + return []; + } + async sourceLocationToRawLocation(): Promise { + return []; + } + async rawLocationToSourceLocation(): Promise { + return []; + } + async getScopeInfo(): Promise { + throw new Error('Method not implemented.'); + } + async listVariablesInScope(): Promise { + return []; + } + async removeRawModule(): Promise { + } + async getFunctionInfo(): Promise<{frames: Array, missingSymbolFiles: Array}| + {missingSymbolFiles: Array}|{frames: Array}> { + return {frames: []}; + } + async getInlinedFunctionRanges(): Promise { + return []; + } + async getInlinedCalleesRanges(): Promise { + return []; + } + async getMappedLines(): Promise { + return undefined; + } + async evaluate(): Promise { + return null; + } + async getProperties(): Promise { + return []; + } + async releaseObject(): Promise { + } +} + describeWithDevtoolsExtension('Language Extension API', {}, context => { it('reports loaded resources', async () => { const target = createTarget(); @@ -814,3 +855,47 @@ describeWithDevtoolsExtension('Language Extension API', {}, context => { assert.deepEqual(resource, expectedResource); }); }); + +for (const allowFileAccess of [true, false]) { + describeWithDevtoolsExtension( + `Language Extension API with {allowFileAccess: ${allowFileAccess}}`, {allowFileAccess}, context => { + let target: SDK.Target.Target; + beforeEach(() => { + target = createTarget(); + const targetManager = target.targetManager(); + const workspace = Workspace.Workspace.WorkspaceImpl.instance(); + const resourceMapping = new Bindings.ResourceMapping.ResourceMapping(targetManager, workspace); + target.setInspectedURL('http://example.com' as Platform.DevToolsPath.UrlString); + const debuggerWorkspaceBinding = Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance( + {forceNew: true, targetManager, resourceMapping}); + Bindings.IgnoreListManager.IgnoreListManager.instance({forceNew: true, debuggerWorkspaceBinding}); + }); + + it('passes allowFileAccess to the LanguageExtensionEndpoint', async () => { + const endpointSpy = + sinon.spy(Extensions.LanguageExtensionEndpoint.LanguageExtensionEndpoint.prototype, 'handleScript'); + const plugin = new StubLanguageExtension(); + await context.chrome.devtools?.languageServices.registerLanguageExtensionPlugin(plugin, 'plugin', { + language: Protocol.Debugger.ScriptLanguage.JavaScript, + symbol_types: [Protocol.Debugger.DebugSymbolsType.SourceMap], + }); + + const debuggerModel = target.model(SDK.DebuggerModel.DebuggerModel); + assert.isOk(debuggerModel); + debuggerModel.parsedScriptSource( + '0' as Protocol.Runtime.ScriptId, 'file:///source/url' as Platform.DevToolsPath.UrlString, 0, 0, 100, 100, + 0, '', {}, false, 'file:///source/url.map', false, false, 200, true, null, null, + Protocol.Debugger.ScriptLanguage.JavaScript, [{ + type: Protocol.Debugger.DebugSymbolsType.SourceMap, + externalURL: 'file:///source/url.map', + }], + null); + + assert.isTrue(endpointSpy.calledOnce); + assert.strictEqual( + (endpointSpy.thisValues[0] as Extensions.LanguageExtensionEndpoint.LanguageExtensionEndpoint) + .allowFileAccess, + allowFileAccess); + }); + }); +} diff --git a/front_end/models/extensions/ExtensionServer.ts b/front_end/models/extensions/ExtensionServer.ts index 7b5a997281..f994738683 100644 --- a/front_end/models/extensions/ExtensionServer.ts +++ b/front_end/models/extensions/ExtensionServer.ts @@ -313,8 +313,12 @@ export class ExtensionServer extends Common.ObjectWrapper.ObjectWrapper typeof e === 'string') ? symbol_types : []); const extensionOrigin = this.getExtensionOrigin(_shared_port); - const endpoint = - new LanguageExtensionEndpoint(extensionOrigin, pluginName, {language, symbol_types: symbol_types_array}, port); + const registration = this.registeredExtensions.get(extensionOrigin); + if (!registration) { + throw new Error('Received a message from an unregistered extension'); + } + const endpoint = new LanguageExtensionEndpoint( + registration.allowFileAccess, extensionOrigin, pluginName, {language, symbol_types: symbol_types_array}, port); pluginManager.addPlugin(endpoint); return this.status.OK(); } diff --git a/front_end/models/extensions/LanguageExtensionEndpoint.test.ts b/front_end/models/extensions/LanguageExtensionEndpoint.test.ts new file mode 100644 index 0000000000..95454a6ae3 --- /dev/null +++ b/front_end/models/extensions/LanguageExtensionEndpoint.test.ts @@ -0,0 +1,59 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import type * as Platform from '../../core/platform/platform.js'; +import * as SDK from '../../core/sdk/sdk.js'; +import * as Protocol from '../../generated/protocol.js'; + +import * as Extensions from './extensions.js'; + +for (const allowFileAccess of [true, false]) { + describe(`LanguageExtensionEndpoint ${allowFileAccess ? 'with' : 'without'} file access`, () => { + let endpoint: Extensions.LanguageExtensionEndpoint.LanguageExtensionEndpoint; + beforeEach(() => { + const channel = new MessageChannel(); + endpoint = new Extensions.LanguageExtensionEndpoint.LanguageExtensionEndpoint( + allowFileAccess, '', '', {language: 'lang', symbol_types: [Protocol.Debugger.DebugSymbolsType.SourceMap]}, + channel.port1); + }); + + it('canAccessURL respects allowFileAccess correctly', () => { + assert.isTrue(endpoint.canAccessURL('http://example.com')); + assert.strictEqual(endpoint.canAccessURL('file:///file'), allowFileAccess); + }); + + it('handleScript respects allowFileAccess correctly', () => { + const script = sinon.createStubInstance(SDK.Script.Script); + script.debugSymbols = {type: Protocol.Debugger.DebugSymbolsType.SourceMap}; + script.scriptLanguage.returns('lang'); + + script.contentURL.returns('file:///file' as Platform.DevToolsPath.UrlString); + assert.strictEqual(endpoint.handleScript(script), allowFileAccess); + script.contentURL.returns('http://example.com' as Platform.DevToolsPath.UrlString); + assert.isTrue(endpoint.handleScript(script)); + + script.hasSourceURL = true; + script.sourceURL = 'file:///file' as Platform.DevToolsPath.UrlString; + assert.strictEqual(endpoint.handleScript(script), allowFileAccess); + script.sourceURL = 'http://example.com' as Platform.DevToolsPath.UrlString; + assert.isTrue(endpoint.handleScript(script)); + + script.debugSymbols.externalURL = 'file:///file'; + assert.strictEqual(endpoint.handleScript(script), allowFileAccess); + script.debugSymbols.externalURL = 'http://example.com'; + assert.isTrue(endpoint.handleScript(script)); + }); + + it('addRawModule respects allowFileAccess correctly', async () => { + const endpointProxyStub = sinon.stub(Extensions.ExtensionEndpoint.ExtensionEndpoint.prototype, 'sendRequest'); + endpointProxyStub.resolves([]); + await endpoint.addRawModule('', 'file:///file', {url: 'http://example.com'}); + assert.strictEqual(endpointProxyStub.calledOnce, allowFileAccess); + await endpoint.addRawModule('', 'http://example.com', {url: 'file:///file'}); + assert.strictEqual(endpointProxyStub.calledTwice, allowFileAccess); + await endpoint.addRawModule('', 'http://example.com', {url: 'http://example.com'}); + assert.lengthOf(endpointProxyStub.getCalls(), allowFileAccess ? 3 : 1); + }); + }); +} diff --git a/front_end/models/extensions/LanguageExtensionEndpoint.ts b/front_end/models/extensions/LanguageExtensionEndpoint.ts index 186903cc2d..d9ac8c26de 100644 --- a/front_end/models/extensions/LanguageExtensionEndpoint.ts +++ b/front_end/models/extensions/LanguageExtensionEndpoint.ts @@ -31,18 +31,17 @@ class LanguageExtensionEndpointImpl extends ExtensionEndpoint { export class LanguageExtensionEndpoint implements Bindings.DebuggerLanguagePlugins.DebuggerLanguagePlugin { private readonly supportedScriptTypes: { language: string, - // TODO(crbug.com/1172300) Ignored during the jsdoc to ts migration // eslint-disable-next-line @typescript-eslint/naming-convention symbol_types: Array, }; - private endpoint: LanguageExtensionEndpointImpl; - private extensionOrigin: string; - name: string; + private readonly endpoint: LanguageExtensionEndpointImpl; + private readonly extensionOrigin: string; + readonly allowFileAccess: boolean; + readonly name: string; constructor( - extensionOrigin: string, name: string, supportedScriptTypes: { + allowFileAccess: boolean, extensionOrigin: string, name: string, supportedScriptTypes: { language: string, - // TODO(crbug.com/1172300) Ignored during the jsdoc to ts migration // eslint-disable-next-line @typescript-eslint/naming-convention symbol_types: Array, }, @@ -51,9 +50,26 @@ export class LanguageExtensionEndpoint implements Bindings.DebuggerLanguagePlugi this.extensionOrigin = extensionOrigin; this.supportedScriptTypes = supportedScriptTypes; this.endpoint = new LanguageExtensionEndpointImpl(this, port); + this.allowFileAccess = allowFileAccess; + } + + canAccessURL(url: string): boolean { + try { + return this.allowFileAccess || new URL(url).protocol !== 'file:'; + } catch (e) { + return false; + } } handleScript(script: SDK.Script.Script): boolean { + try { + if (!this.canAccessURL(script.contentURL()) || (script.hasSourceURL && !this.canAccessURL(script.sourceURL)) || + (script.debugSymbols?.externalURL && !this.canAccessURL(script.debugSymbols.externalURL))) { + return false; + } + } catch (e) { + return false; + } const language = script.scriptLanguage(); return language !== null && script.debugSymbols !== null && language === this.supportedScriptTypes.language && this.supportedScriptTypes.symbol_types.includes(script.debugSymbols.type); @@ -71,6 +87,9 @@ export class LanguageExtensionEndpoint implements Bindings.DebuggerLanguagePlugi /** Notify the plugin about a new script */ addRawModule(rawModuleId: string, symbolsURL: string, rawModule: Chrome.DevTools.RawModule): Promise { + if (!this.canAccessURL(symbolsURL) || !this.canAccessURL(rawModule.url)) { + return Promise.resolve([]); + } return this.endpoint.sendRequest( PrivateAPI.LanguageExtensionPluginCommands.AddRawModule, {rawModuleId, symbolsURL, rawModule}) as Promise; diff --git a/front_end/models/extensions/extensions.ts b/front_end/models/extensions/extensions.ts index ffde704173..1a953fda5c 100644 --- a/front_end/models/extensions/extensions.ts +++ b/front_end/models/extensions/extensions.ts @@ -3,19 +3,23 @@ // found in the LICENSE file. import * as ExtensionAPI from './ExtensionAPI.js'; +import * as ExtensionEndpoint from './ExtensionEndpoint.js'; import * as ExtensionPanel from './ExtensionPanel.js'; import * as ExtensionServer from './ExtensionServer.js'; import * as ExtensionView from './ExtensionView.js'; import * as HostUrlPattern from './HostUrlPattern.js'; +import * as LanguageExtensionEndpoint from './LanguageExtensionEndpoint.js'; import * as RecorderExtensionEndpoint from './RecorderExtensionEndpoint.js'; import * as RecorderPluginManager from './RecorderPluginManager.js'; export { ExtensionAPI, + ExtensionEndpoint, ExtensionPanel, ExtensionServer, ExtensionView, HostUrlPattern, + LanguageExtensionEndpoint, RecorderExtensionEndpoint, RecorderPluginManager, }; diff --git a/test/e2e/extensions/debugger-language-plugins_test.ts b/test/e2e/extensions/debugger-language-plugins_test.ts index 45b9e4ed20..cc2c304a9d 100644 --- a/test/e2e/extensions/debugger-language-plugins_test.ts +++ b/test/e2e/extensions/debugger-language-plugins_test.ts @@ -98,7 +98,8 @@ describe('The Debugger Language Plugins', () => { it('can show C filenames after loading the module', async () => { const {target} = getBrowserAndPages(); const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess*/ true); await extension.evaluate(() => { // A simple plugin that resolves to a single source file class SingleFilePlugin { @@ -129,7 +130,8 @@ describe('The Debugger Language Plugins', () => { // Resolve a single code offset to a source line to test the correctness of offset computations. it('use correct code offsets to interpret raw locations', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); const locationLabels = WasmLocationLabels.load('extensions/unreachable.wat', 'extensions/unreachable.wasm'); await extension.evaluate((mappings: LabelMapping[]) => { class LocationMappingPlugin { @@ -186,7 +188,8 @@ describe('The Debugger Language Plugins', () => { it('resolve locations for breakpoints correctly', async () => { const locationLabels = WasmLocationLabels.load('extensions/global_variable.wat', 'extensions/global_variable.wasm'); const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate((mappings: LabelMapping[]) => { // This plugin will emulate a source mapping with a single file and a single corresponding source line and byte // code offset pair. @@ -260,7 +263,8 @@ describe('The Debugger Language Plugins', () => { it('shows top-level and nested variables', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluateHandle(() => { class VariableListingPlugin { private modules: @@ -323,7 +327,8 @@ describe('The Debugger Language Plugins', () => { it('shows inline frames', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class InliningPlugin { private modules: Map { it('falls back to wasm function names when inline info not present', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class InliningPlugin { private modules: Map { it('shows a warning when no debug info is present', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class MissingInfoPlugin { private modules: Map { it('shows warnings when function info not present', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class MissingInfoPlugin { private modules: Map { it('connects warnings to the developer resource panel', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class MissingInfoPlugin { async addRawModule() { @@ -667,7 +676,8 @@ describe('The Debugger Language Plugins', () => { it('shows variable values with the evaluate API', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class EvalPlugin { private modules: @@ -792,7 +802,8 @@ describe('The Debugger Language Plugins', () => { it('shows variable value in popover', async () => { const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class VariableListingPlugin { private modules: @@ -875,7 +886,8 @@ describe('The Debugger Language Plugins', () => { it('shows sensible error messages.', async () => { const {frontend} = getBrowserAndPages(); const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class FormattingErrorsPlugin { private modules: @@ -994,7 +1006,8 @@ describe('The Debugger Language Plugins', () => { it('can access wasm data directly', async () => { const {target} = getBrowserAndPages(); const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { class WasmDataExtension { constructor() { @@ -1066,7 +1079,8 @@ describe('The Debugger Language Plugins', () => { it('lets users manually attach debug info', async () => { const {target} = getBrowserAndPages(); const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); await extension.evaluate(() => { // A simple plugin that resolves to a single source file class DWARFSymbolsWithSingleFilePlugin { @@ -1143,7 +1157,8 @@ describe('The Debugger Language Plugins', () => { it('auto-steps over unmapped code correctly', async () => { const {frontend} = getBrowserAndPages(); const extension = await loadExtension( - 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`); + 'TestExtension', `${getResourcesPathWithDevToolsHostname()}/extensions/language_extensions.html`, + /* allowFileAccess */ true); const locationLabels = WasmLocationLabels.load('extensions/stepping.wat', 'extensions/stepping.wasm'); await goToWasmResource('stepping.wasm', {autoLoadModule: true}); diff --git a/test/e2e/helpers/extension-helpers.ts b/test/e2e/helpers/extension-helpers.ts index ef61b59e06..26857763fc 100644 --- a/test/e2e/helpers/extension-helpers.ts +++ b/test/e2e/helpers/extension-helpers.ts @@ -35,10 +35,10 @@ export function getResourcesPathWithDevToolsHostname() { return getResourcesPath(getDevToolsFrontendHostname()); } -export async function loadExtension(name: string, startPage?: string) { +export async function loadExtension(name: string, startPage?: string, allowFileAccess?: boolean) { startPage = startPage || `${getResourcesPathWithDevToolsHostname()}/extensions/empty_extension.html`; const {frontend} = getBrowserAndPages(); - const extensionInfo = {startPage, name}; + const extensionInfo = {startPage, name, allowFileAccess}; // Because the injected script is shared across calls for the target, we cannot run multiple instances concurrently. const load = loadExtensionPromise.then(() => doLoad(frontend, extensionInfo));