Browse Source

feat: enable sandbox by default in limited circumstances (#30197)

Jeremy Rose 3 years ago
parent
commit
8179349625

+ 17 - 4
shell/browser/web_contents_preferences.cc

@@ -21,6 +21,7 @@
 #include "sandbox/policy/switches.h"
 #include "shell/browser/api/electron_api_web_contents.h"
 #include "shell/browser/native_window.h"
+#include "shell/browser/session_preferences.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/options_switches.h"
@@ -127,7 +128,7 @@ void WebContentsPreferences::Clear() {
   node_integration_in_worker_ = false;
   disable_html_fullscreen_window_resize_ = false;
   webview_tag_ = false;
-  sandbox_ = false;
+  sandbox_ = absl::nullopt;
   native_window_open_ = true;
   context_isolation_ = true;
   javascript_ = true;
@@ -185,7 +186,9 @@ void WebContentsPreferences::SetFromDictionary(
   web_preferences.Get(options::kDisableHtmlFullscreenWindowResize,
                       &disable_html_fullscreen_window_resize_);
   web_preferences.Get(options::kWebviewTag, &webview_tag_);
-  web_preferences.Get(options::kSandbox, &sandbox_);
+  bool sandbox;
+  if (web_preferences.Get(options::kSandbox, &sandbox))
+    sandbox_ = sandbox;
   web_preferences.Get(options::kNativeWindowOpen, &native_window_open_);
   web_preferences.Get(options::kContextIsolation, &context_isolation_);
   web_preferences.Get(options::kJavaScript, &javascript_);
@@ -307,6 +310,16 @@ bool WebContentsPreferences::GetPreloadPath(base::FilePath* path) const {
   return false;
 }
 
+bool WebContentsPreferences::IsSandboxed() const {
+  if (sandbox_)
+    return *sandbox_;
+  bool sandbox_disabled_by_default =
+      node_integration_ || node_integration_in_worker_ || preload_path_ ||
+      !SessionPreferences::GetValidPreloads(web_contents_->GetBrowserContext())
+           .empty();
+  return !sandbox_disabled_by_default;
+}
+
 // static
 content::WebContents* WebContentsPreferences::GetWebContentsFromProcessID(
     int process_id) {
@@ -338,7 +351,7 @@ void WebContentsPreferences::AppendCommandLineSwitches(
   // unless nodeIntegrationInSubFrames is enabled
   bool can_sandbox_frame = is_subframe && !node_integration_in_sub_frames_;
 
-  if (sandbox_ || can_sandbox_frame) {
+  if (IsSandboxed() || can_sandbox_frame) {
     command_line->AppendSwitch(switches::kEnableSandbox);
   } else if (!command_line->HasSwitch(switches::kEnableSandbox)) {
     command_line->AppendSwitch(sandbox::policy::switches::kNoSandbox);
@@ -387,7 +400,7 @@ void WebContentsPreferences::SaveLastPreferences() {
                                base::Value(node_integration_in_sub_frames_));
   last_web_preferences_.SetKey(options::kNativeWindowOpen,
                                base::Value(native_window_open_));
-  last_web_preferences_.SetKey(options::kSandbox, base::Value(sandbox_));
+  last_web_preferences_.SetKey(options::kSandbox, base::Value(IsSandboxed()));
   last_web_preferences_.SetKey(options::kContextIsolation,
                                base::Value(context_isolation_));
   last_web_preferences_.SetKey(options::kJavaScript, base::Value(javascript_));

+ 2 - 1
shell/browser/web_contents_preferences.h

@@ -68,6 +68,7 @@ class WebContentsPreferences
   bool ShouldUseNativeWindowOpen() const { return native_window_open_; }
   bool IsWebSecurityEnabled() const { return web_security_; }
   bool GetPreloadPath(base::FilePath* path) const;
+  bool IsSandboxed() const;
 
  private:
   friend class content::WebContentsUserData<WebContentsPreferences>;
@@ -88,7 +89,7 @@ class WebContentsPreferences
   bool node_integration_in_worker_;
   bool disable_html_fullscreen_window_resize_;
   bool webview_tag_;
-  bool sandbox_;
+  absl::optional<bool> sandbox_;
   bool native_window_open_;
   bool context_isolation_;
   bool javascript_;

+ 20 - 15
spec-main/fixtures/snapshots/proxy-window-open.snapshot.txt

@@ -2,7 +2,9 @@
   [
     "top=5,left=10,resizable=no",
     {
-      "sender": "[WebContents]"
+      "sender": "[WebContents]",
+      "frameId": 1,
+      "processId": "placeholder-process-id"
     },
     "about:blank",
     "frame-name",
@@ -18,11 +20,10 @@
       "y": 5,
       "webPreferences": {
         "contextIsolation": true,
-        "nativeWindowOpen": true,
         "nodeIntegration": false,
         "webviewTag": false,
         "nodeIntegrationInSubFrames": false,
-        "openerId": null
+        "openerId": "placeholder-opener-id"
       },
       "webContents": "[WebContents]"
     },
@@ -36,7 +37,9 @@
   [
     "zoomFactor=2,resizable=0,x=0,y=10",
     {
-      "sender": "[WebContents]"
+      "sender": "[WebContents]",
+      "frameId": 1,
+      "processId": "placeholder-process-id"
     },
     "about:blank",
     "frame-name",
@@ -51,11 +54,10 @@
       "webPreferences": {
         "zoomFactor": "2",
         "contextIsolation": true,
-        "nativeWindowOpen": true,
         "nodeIntegration": false,
         "webviewTag": false,
         "nodeIntegrationInSubFrames": false,
-        "openerId": null
+        "openerId": "placeholder-opener-id"
       },
       "webContents": "[WebContents]"
     },
@@ -69,7 +71,9 @@
   [
     "backgroundColor=gray,webPreferences=0,x=100,y=100",
     {
-      "sender": "[WebContents]"
+      "sender": "[WebContents]",
+      "frameId": 1,
+      "processId": "placeholder-process-id"
     },
     "about:blank",
     "frame-name",
@@ -81,11 +85,10 @@
       "backgroundColor": "gray",
       "webPreferences": {
         "contextIsolation": true,
-        "nativeWindowOpen": true,
         "nodeIntegration": false,
         "webviewTag": false,
         "nodeIntegrationInSubFrames": false,
-        "openerId": null,
+        "openerId": "placeholder-opener-id",
         "backgroundColor": "gray"
       },
       "x": 100,
@@ -102,7 +105,9 @@
   [
     "x=50,y=20,title=sup",
     {
-      "sender": "[WebContents]"
+      "sender": "[WebContents]",
+      "frameId": 1,
+      "processId": "placeholder-process-id"
     },
     "about:blank",
     "frame-name",
@@ -116,11 +121,10 @@
       "title": "sup",
       "webPreferences": {
         "contextIsolation": true,
-        "nativeWindowOpen": true,
         "nodeIntegration": false,
         "webviewTag": false,
         "nodeIntegrationInSubFrames": false,
-        "openerId": null
+        "openerId": "placeholder-opener-id"
       },
       "webContents": "[WebContents]"
     },
@@ -134,7 +138,9 @@
   [
     "show=false,top=1,left=1",
     {
-      "sender": "[WebContents]"
+      "sender": "[WebContents]",
+      "frameId": 1,
+      "processId": "placeholder-process-id"
     },
     "about:blank",
     "frame-name",
@@ -149,11 +155,10 @@
       "y": 1,
       "webPreferences": {
         "contextIsolation": true,
-        "nativeWindowOpen": true,
         "nodeIntegration": false,
         "webviewTag": false,
         "nodeIntegrationInSubFrames": false,
-        "openerId": null
+        "openerId": "placeholder-opener-id"
       },
       "webContents": "[WebContents]"
     },

+ 5 - 1
spec-main/guest-window-manager-spec.ts

@@ -33,7 +33,11 @@ describe('new-window event', () => {
     proxy: {
       snapshotFileName: 'proxy-window-open.snapshot.txt',
       browserWindowOptions: {
-        show: false
+        show: false,
+        webPreferences: {
+          nativeWindowOpen: false,
+          sandbox: false
+        }
       }
     }
   };