Browse Source

fix: do not trigger CSP violations when checking eval (#30991)

* fix: do not trigger CSP violations when checking eval

* Update shell/renderer/api/electron_api_web_frame.cc

Co-authored-by: Cheng Zhao <[email protected]>

Co-authored-by: Cheng Zhao <[email protected]>
Samuel Attard 3 years ago
parent
commit
63eed52626

+ 8 - 17
lib/renderer/security-warnings.ts

@@ -77,15 +77,8 @@ const isLocalhost = function () {
  *
  * @returns {boolean} Is a CSP with `unsafe-eval` set?
  */
-const isUnsafeEvalEnabled: () => Promise<boolean> = function () {
-  return webFrame.executeJavaScript(`(${(() => {
-    try {
-      eval(window.trustedTypes.emptyScript); // eslint-disable-line no-eval
-    } catch {
-      return false;
-    }
-    return true;
-  }).toString()})()`, false);
+const isUnsafeEvalEnabled = () => {
+  return webFrame._isEvalAllowed();
 };
 
 const moreInformation = `\nFor more information and help, consult
@@ -174,16 +167,14 @@ const warnAboutDisabledWebSecurity = function (webPreferences?: Electron.WebPref
  * Logs a warning message about unset or insecure CSP
  */
 const warnAboutInsecureCSP = function () {
-  isUnsafeEvalEnabled().then((enabled) => {
-    if (!enabled) return;
+  if (!isUnsafeEvalEnabled()) return;
 
-    const warning = `This renderer process has either no Content Security
-    Policy set or a policy with "unsafe-eval" enabled. This exposes users of
-    this app to unnecessary security risks.\n${moreInformation}`;
+  const warning = `This renderer process has either no Content Security
+  Policy set or a policy with "unsafe-eval" enabled. This exposes users of
+  this app to unnecessary security risks.\n${moreInformation}`;
 
-    console.warn('%cElectron Security Warning (Insecure Content-Security-Policy)',
-      'font-weight: bold;', warning);
-  }).catch(() => {});
+  console.warn('%cElectron Security Warning (Insecure Content-Security-Policy)',
+    'font-weight: bold;', warning);
 };
 
 /**

+ 13 - 0
shell/renderer/api/electron_api_web_frame.cc

@@ -49,6 +49,8 @@
 #include "third_party/blink/public/web/web_script_execution_callback.h"
 #include "third_party/blink/public/web/web_script_source.h"
 #include "third_party/blink/public/web/web_view.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"  // nogncheck
+#include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"  // nogncheck
 #include "third_party/blink/renderer/platform/bindings/dom_wrapper_world.h"  // nogncheck
 #include "ui/base/ime/ime_text_span.h"
 #include "url/url_util.h"
@@ -369,6 +371,7 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
         .SetMethod("insertText", &WebFrameRenderer::InsertText)
         .SetMethod("insertCSS", &WebFrameRenderer::InsertCSS)
         .SetMethod("removeInsertedCSS", &WebFrameRenderer::RemoveInsertedCSS)
+        .SetMethod("_isEvalAllowed", &WebFrameRenderer::IsEvalAllowed)
         .SetMethod("executeJavaScript", &WebFrameRenderer::ExecuteJavaScript)
         .SetMethod("executeJavaScriptInIsolatedWorld",
                    &WebFrameRenderer::ExecuteJavaScriptInIsolatedWorld)
@@ -637,6 +640,16 @@ class WebFrameRenderer : public gin::Wrappable<WebFrameRenderer>,
     }
   }
 
+  bool IsEvalAllowed(v8::Isolate* isolate) {
+    content::RenderFrame* render_frame;
+    if (!MaybeGetRenderFrame(isolate, "isEvalAllowed", &render_frame))
+      return true;
+
+    auto* context = blink::ExecutionContext::From(
+        render_frame->GetWebFrame()->MainWorldScriptContext());
+    return !context->GetContentSecurityPolicy()->ShouldCheckEval();
+  }
+
   v8::Local<v8::Promise> ExecuteJavaScript(gin::Arguments* gin_args,
                                            const std::u16string& code) {
     gin_helper::Arguments* args = static_cast<gin_helper::Arguments*>(gin_args);

+ 10 - 9
spec-main/security-warnings-spec.ts

@@ -9,6 +9,7 @@ import { BrowserWindow, WebPreferences } from 'electron/main';
 import { closeWindow } from './window-helpers';
 import { AddressInfo } from 'net';
 import { emittedUntil } from './events-helpers';
+import { delay } from './spec-helpers';
 
 const messageContainsSecurityWarning = (event: Event, level: number, message: string) => {
   return message.indexOf('Electron Security Warning') > -1;
@@ -22,7 +23,6 @@ describe('security warnings', () => {
   let server: http.Server;
   let w: BrowserWindow;
   let useCsp = true;
-  let useTrustedTypes = false;
   let serverUrl: string;
 
   before((done) => {
@@ -50,8 +50,7 @@ describe('security warnings', () => {
           }
 
           const cspHeaders = [
-            ...(useCsp ? ['script-src \'self\' \'unsafe-inline\''] : []),
-            ...(useTrustedTypes ? ['require-trusted-types-for \'script\'; trusted-types *'] : [])
+            ...(useCsp ? ['script-src \'self\' \'unsafe-inline\''] : [])
           ];
           response.writeHead(200, { 'Content-Security-Policy': cspHeaders });
           response.write(file, 'binary');
@@ -72,7 +71,6 @@ describe('security warnings', () => {
 
   afterEach(async () => {
     useCsp = true;
-    useTrustedTypes = false;
     await closeWindow(w);
     w = null as unknown as any;
   });
@@ -132,17 +130,20 @@ describe('security warnings', () => {
         expect(message).to.include('Insecure Content-Security-Policy');
       });
 
-      it('should warn about insecure Content-Security-Policy (Trusted Types)', async () => {
+      it('should not warn about secure Content-Security-Policy', async () => {
         w = new BrowserWindow({
           show: false,
           webPreferences
         });
 
-        useCsp = false;
-        useTrustedTypes = true;
+        useCsp = true;
         w.loadURL(`${serverUrl}/base-page-security.html`);
-        const [,, message] = await emittedUntil(w.webContents, 'console-message', messageContainsSecurityWarning);
-        expect(message).to.include('Insecure Content-Security-Policy');
+        let didNotWarn = true;
+        w.webContents.on('console-message', () => {
+          didNotWarn = false;
+        });
+        await delay(500);
+        expect(didNotWarn).to.equal(true);
       });
 
       it('should warn about allowRunningInsecureContent', async () => {

+ 4 - 0
typings/internal-electron.d.ts

@@ -90,6 +90,10 @@ declare namespace Electron {
     _postMessage(channel: string, message: any, transfer?: any[]): void;
   }
 
+  interface WebFrame {
+    _isEvalAllowed(): boolean;
+  }
+
   interface WebPreferences {
     openerId?: number | null;
     disablePopups?: boolean;