Skip to content

Commit

Permalink
refactor: clean up webFrame implementation to use gin wrappers
Browse files Browse the repository at this point in the history
The previous implementation of webFrame in the renderer process leaked
sub-frame contexts and global objects across the context boundaries thus
making it possible for apps to either maliciously or accidentally
violate the contextIsolation boundary.

This re-implementation binds all methods in native code directly to
content::RenderFrame instances instead of relying on JS to provide a
"window" with every method request.  This is much more consistent with
the rest of the Electron codebase and is substantially safer.
  • Loading branch information
MarshallOfSound committed Apr 2, 2021
1 parent 55c66e3 commit f69ac0d
Show file tree
Hide file tree
Showing 10 changed files with 609 additions and 646 deletions.
4 changes: 2 additions & 2 deletions lib/renderer/api/context-bridge.ts
@@ -1,7 +1,7 @@
const { getWebPreference } = process._linkedBinding('electron_renderer_web_frame');
const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');
const binding = process._linkedBinding('electron_renderer_context_bridge');

const contextIsolationEnabled = getWebPreference(window, 'contextIsolation');
const contextIsolationEnabled = mainFrame.getWebPreference('contextIsolation');

const checkContextIsolationEnabled = () => {
if (!contextIsolationEnabled) throw new Error('contextBridge API can only be used when contextIsolation is enabled');
Expand Down
85 changes: 2 additions & 83 deletions lib/renderer/api/web-frame.ts
@@ -1,84 +1,3 @@
import { EventEmitter } from 'events';
import deprecate from '@electron/internal/common/api/deprecate';
const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');

const binding = process._linkedBinding('electron_renderer_web_frame');

class WebFrame extends EventEmitter {
constructor (public context: Window) {
super();

// Lots of webview would subscribe to webFrame's events.
this.setMaxListeners(0);
}

findFrameByRoutingId (routingId: number) {
return getWebFrame(binding._findFrameByRoutingId(this.context, routingId));
}

getFrameForSelector (selector: string) {
return getWebFrame(binding._getFrameForSelector(this.context, selector));
}

findFrameByName (name: string) {
return getWebFrame(binding._findFrameByName(this.context, name));
}

get opener () {
return getWebFrame(binding._getOpener(this.context));
}

get parent () {
return getWebFrame(binding._getParent(this.context));
}

get top () {
return getWebFrame(binding._getTop(this.context));
}

get firstChild () {
return getWebFrame(binding._getFirstChild(this.context));
}

get nextSibling () {
return getWebFrame(binding._getNextSibling(this.context));
}

get routingId () {
return binding._getRoutingId(this.context);
}
}

const contextIsolation = binding.getWebPreference(window, 'contextIsolation');
const worldSafeExecuteJavaScript = binding.getWebPreference(window, 'worldSafeExecuteJavaScript');

const worldSafeJS = worldSafeExecuteJavaScript || !contextIsolation;

// Populate the methods.
for (const name in binding) {
if (!name.startsWith('_')) { // some methods are manually populated above
// TODO(felixrieseberg): Once we can type web_frame natives, we could
// use a neat `keyof` here
(WebFrame as any).prototype[name] = function (...args: Array<any>) {
if (!worldSafeJS && name.startsWith('executeJavaScript')) {
deprecate.log(`Security Warning: webFrame.${name} was called without worldSafeExecuteJavaScript enabled. This is considered unsafe. worldSafeExecuteJavaScript will be enabled by default in Electron 12.`);
}
return (binding as any)[name](this.context, ...args);
};
// TODO(MarshallOfSound): Remove once the above deprecation is removed
if (name.startsWith('executeJavaScript')) {
(WebFrame as any).prototype[`_${name}`] = function (...args: Array<any>) {
return (binding as any)[name](this.context, ...args);
};
}
}
}

// Helper to return WebFrame or null depending on context.
// TODO(zcbenz): Consider returning same WebFrame for the same frame.
function getWebFrame (context: Window) {
return context ? new WebFrame(context) : null;
}

const _webFrame = new WebFrame(window);

export default _webFrame;
export default mainFrame;
24 changes: 12 additions & 12 deletions lib/renderer/init.ts
Expand Up @@ -63,18 +63,18 @@ webFrameInit();

// Process command line arguments.
const { hasSwitch, getSwitchValue } = process._linkedBinding('electron_common_command_line');
const { getWebPreference } = process._linkedBinding('electron_renderer_web_frame');

const contextIsolation = getWebPreference(window, 'contextIsolation');
const nodeIntegration = getWebPreference(window, 'nodeIntegration');
const webviewTag = getWebPreference(window, 'webviewTag');
const isHiddenPage = getWebPreference(window, 'hiddenPage');
const usesNativeWindowOpen = getWebPreference(window, 'nativeWindowOpen');
const rendererProcessReuseEnabled = getWebPreference(window, 'disableElectronSiteInstanceOverrides');
const preloadScript = getWebPreference(window, 'preload');
const preloadScripts = getWebPreference(window, 'preloadScripts');
const guestInstanceId = getWebPreference(window, 'guestInstanceId') || null;
const openerId = getWebPreference(window, 'openerId') || null;
const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');

const contextIsolation = mainFrame.getWebPreference('contextIsolation');
const nodeIntegration = mainFrame.getWebPreference('nodeIntegration');
const webviewTag = mainFrame.getWebPreference('webviewTag');
const isHiddenPage = mainFrame.getWebPreference('hiddenPage');
const usesNativeWindowOpen = mainFrame.getWebPreference('nativeWindowOpen');
const rendererProcessReuseEnabled = mainFrame.getWebPreference('disableElectronSiteInstanceOverrides');
const preloadScript = mainFrame.getWebPreference('preload');
const preloadScripts = mainFrame.getWebPreference('preloadScripts');
const guestInstanceId = mainFrame.getWebPreference('guestInstanceId') || null;
const openerId = mainFrame.getWebPreference('openerId') || null;
const appPath = hasSwitch('app-path') ? getSwitchValue('app-path') : null;

// The webContents preload script is loaded after the session preload scripts.
Expand Down
2 changes: 1 addition & 1 deletion lib/renderer/security-warnings.ts
Expand Up @@ -79,7 +79,7 @@ const isLocalhost = function () {
*/
const isUnsafeEvalEnabled: () => Promise<boolean> = function () {
// Call _executeJavaScript to bypass the world-safe deprecation warning
return webFrame._executeJavaScript(`(${(() => {
return webFrame.executeJavaScript(`(${(() => {
try {
eval(window.trustedTypes.emptyScript); // eslint-disable-line no-eval
} catch {
Expand Down
5 changes: 0 additions & 5 deletions lib/renderer/web-frame-init.ts
Expand Up @@ -13,11 +13,6 @@ export const webFrameInit = () => {
ipcRendererUtils.handle(IPC_MESSAGES.RENDERER_WEB_FRAME_METHOD, (
event, method: keyof WebFrameMethod, ...args: any[]
) => {
// TODO(MarshallOfSound): Remove once the world-safe-execute-javascript deprecation warning is removed
if (method.startsWith('executeJavaScript')) {
return (webFrame as any)[`_${method}`](...args);
}

// The TypeScript compiler cannot handle the sheer number of
// call signatures here and simply gives up. Incorrect invocations
// will be caught by "keyof WebFrameMethod" though.
Expand Down
14 changes: 7 additions & 7 deletions lib/sandboxed_renderer/init.ts
Expand Up @@ -113,21 +113,21 @@ function preloadRequire (module: string) {

// Process command line arguments.
const { hasSwitch } = process._linkedBinding('electron_common_command_line');
const { getWebPreference } = process._linkedBinding('electron_renderer_web_frame');
const { mainFrame } = process._linkedBinding('electron_renderer_web_frame');

// Similar to nodes --expose-internals flag, this exposes _linkedBinding so
// that tests can call it to get access to some test only bindings
if (hasSwitch('unsafely-expose-electron-internals-for-testing')) {
preloadProcess._linkedBinding = process._linkedBinding;
}

const contextIsolation = getWebPreference(window, 'contextIsolation');
const webviewTag = getWebPreference(window, 'webviewTag');
const isHiddenPage = getWebPreference(window, 'hiddenPage');
const rendererProcessReuseEnabled = getWebPreference(window, 'disableElectronSiteInstanceOverrides');
const contextIsolation = mainFrame.getWebPreference('contextIsolation');
const webviewTag = mainFrame.getWebPreference('webviewTag');
const isHiddenPage = mainFrame.getWebPreference('hiddenPage');
const rendererProcessReuseEnabled = mainFrame.getWebPreference('disableElectronSiteInstanceOverrides');
const usesNativeWindowOpen = true;
const guestInstanceId = getWebPreference(window, 'guestInstanceId') || null;
const openerId = getWebPreference(window, 'openerId') || null;
const guestInstanceId = mainFrame.getWebPreference('guestInstanceId') || null;
const openerId = mainFrame.getWebPreference('openerId') || null;

switch (window.location.protocol) {
case 'devtools:': {
Expand Down
12 changes: 6 additions & 6 deletions shell/common/gin_helper/error_thrower.cc
Expand Up @@ -17,27 +17,27 @@ ErrorThrower::ErrorThrower() : isolate_(v8::Isolate::GetCurrent()) {}

ErrorThrower::~ErrorThrower() = default;

void ErrorThrower::ThrowError(base::StringPiece err_msg) {
void ErrorThrower::ThrowError(base::StringPiece err_msg) const {
Throw(v8::Exception::Error, err_msg);
}

void ErrorThrower::ThrowTypeError(base::StringPiece err_msg) {
void ErrorThrower::ThrowTypeError(base::StringPiece err_msg) const {
Throw(v8::Exception::TypeError, err_msg);
}

void ErrorThrower::ThrowRangeError(base::StringPiece err_msg) {
void ErrorThrower::ThrowRangeError(base::StringPiece err_msg) const {
Throw(v8::Exception::RangeError, err_msg);
}

void ErrorThrower::ThrowReferenceError(base::StringPiece err_msg) {
void ErrorThrower::ThrowReferenceError(base::StringPiece err_msg) const {
Throw(v8::Exception::ReferenceError, err_msg);
}

void ErrorThrower::ThrowSyntaxError(base::StringPiece err_msg) {
void ErrorThrower::ThrowSyntaxError(base::StringPiece err_msg) const {
Throw(v8::Exception::SyntaxError, err_msg);
}

void ErrorThrower::Throw(ErrorGenerator gen, base::StringPiece err_msg) {
void ErrorThrower::Throw(ErrorGenerator gen, base::StringPiece err_msg) const {
v8::Local<v8::Value> exception = gen(gin::StringToV8(isolate_, err_msg));
if (!isolate_->IsExecutionTerminating())
isolate_->ThrowException(exception);
Expand Down
12 changes: 6 additions & 6 deletions shell/common/gin_helper/error_thrower.h
Expand Up @@ -17,18 +17,18 @@ class ErrorThrower {

~ErrorThrower();

void ThrowError(base::StringPiece err_msg);
void ThrowTypeError(base::StringPiece err_msg);
void ThrowRangeError(base::StringPiece err_msg);
void ThrowReferenceError(base::StringPiece err_msg);
void ThrowSyntaxError(base::StringPiece err_msg);
void ThrowError(base::StringPiece err_msg) const;
void ThrowTypeError(base::StringPiece err_msg) const;
void ThrowRangeError(base::StringPiece err_msg) const;
void ThrowReferenceError(base::StringPiece err_msg) const;
void ThrowSyntaxError(base::StringPiece err_msg) const;

v8::Isolate* isolate() const { return isolate_; }

private:
using ErrorGenerator =
v8::Local<v8::Value> (*)(v8::Local<v8::String> err_msg);
void Throw(ErrorGenerator gen, base::StringPiece err_msg);
void Throw(ErrorGenerator gen, base::StringPiece err_msg) const;

v8::Isolate* isolate_;
};
Expand Down

0 comments on commit f69ac0d

Please sign in to comment.