Title: [273965] trunk/Source/WebKit
Revision
273965
Author
commit-qu...@webkit.org
Date
2021-03-05 07:31:50 -0800 (Fri, 05 Mar 2021)

Log Message

[GTK] Bubblewrap sandbox should not break X11 forwarding
https://bugs.webkit.org/show_bug.cgi?id=221990

Patch by Michael Catanzaro <mcatanz...@gnome.org> on 2021-03-05
Reviewed by Carlos Alberto Lopez Perez.

If $DISPLAY points to a TCP socket, or a Unix socket on a different host, then we cannot
isolate the web process from the network and must grant access to the host network
namespace.

Also, clean up some related code by adding PLATFORM(X11) guards where appropriate and
removing a redundant display type check.

* UIProcess/Launcher/glib/BubblewrapLauncher.cpp:
(WebKit::bindWayland):
(WebKit::shouldUnshareNetwork):
(WebKit::bubblewrapSpawn):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (273964 => 273965)


--- trunk/Source/WebKit/ChangeLog	2021-03-05 15:05:07 UTC (rev 273964)
+++ trunk/Source/WebKit/ChangeLog	2021-03-05 15:31:50 UTC (rev 273965)
@@ -1,3 +1,22 @@
+2021-03-05  Michael Catanzaro  <mcatanz...@gnome.org>
+
+        [GTK] Bubblewrap sandbox should not break X11 forwarding
+        https://bugs.webkit.org/show_bug.cgi?id=221990
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        If $DISPLAY points to a TCP socket, or a Unix socket on a different host, then we cannot
+        isolate the web process from the network and must grant access to the host network
+        namespace.
+
+        Also, clean up some related code by adding PLATFORM(X11) guards where appropriate and
+        removing a redundant display type check.
+
+        * UIProcess/Launcher/glib/BubblewrapLauncher.cpp:
+        (WebKit::bindWayland):
+        (WebKit::shouldUnshareNetwork):
+        (WebKit::bubblewrapSpawn):
+
 2021-03-05  Guido Günther  <a...@sigxcpu.org>
 
         Add support for gstreamer's h264 stateless codecs

Modified: trunk/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp (273964 => 273965)


--- trunk/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp	2021-03-05 15:05:07 UTC (rev 273964)
+++ trunk/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp	2021-03-05 15:31:50 UTC (rev 273965)
@@ -327,6 +327,7 @@
     }
 }
 
+#if PLATFORM(X11)
 static void bindX11(Vector<CString>& args)
 {
     const char* display = g_getenv("DISPLAY");
@@ -349,13 +350,11 @@
     } else
         bindIfExists(args, xauth);
 }
+#endif
 
 #if PLATFORM(WAYLAND) && USE(EGL)
 static void bindWayland(Vector<CString>& args)
 {
-    if (PlatformDisplay::sharedDisplay().type() != PlatformDisplay::Type::Wayland)
-        return;
-
     const char* display = g_getenv("WAYLAND_DISPLAY");
     if (!display)
         display = "wayland-0";
@@ -718,6 +717,29 @@
     return tmpfd;
 }
 
+static bool shouldUnshareNetwork(ProcessLauncher::ProcessType processType)
+{
+    // xdg-dbus-proxy needs access to host abstract sockets to connect to the a11y bus. Secure
+    // host services must not use abstract sockets.
+    if (processType == ProcessLauncher::ProcessType::DBusProxy)
+        return false;
+
+#if PLATFORM(X11)
+    // Also, the web process needs access to host networking if the X server is running over TCP or
+    // on a different host's Unix socket; this is likely the case if the first character of DISPLAY
+    // is not a colon.
+    if (processType == ProcessLauncher::ProcessType::Web && PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::X11) {
+        const char* display = g_getenv("DISPLAY");
+        if (display && display[0] != ':')
+            return false;
+    }
+#endif
+
+    // Otherwise, only the network process should have network access. If we are the network
+    // process, then we are not sandboxed and have already bailed out before this point.
+    return true;
+}
+
 GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const ProcessLauncher::LaunchOptions& launchOptions, char** argv, GError **error)
 {
     ASSERT(launcher);
@@ -776,15 +798,11 @@
             // is where we mount our proxy socket.
             "--bind", runDir, runDir,
         }));
-    } else {
-        // xdg-dbus-proxy needs access to host abstract sockets to connect to the a11y bus. Secure
-        // host services must not use abstract sockets. Otherwise, only the network process should
-        // have network access, and the network process is not sandboxed at all.
-        sandboxArgs.appendVector(Vector<CString>({
-            "--unshare-net"
-        }));
     }
 
+    if (shouldUnshareNetwork(launchOptions.processType))
+        sandboxArgs.append("--unshare-net");
+
     // We would have to parse ld config files for more info.
     bindPathVar(sandboxArgs, "LD_LIBRARY_PATH");
 
@@ -817,14 +835,16 @@
     if (launchOptions.processType == ProcessLauncher::ProcessType::Web) {
         static XDGDBusProxyLauncher proxy;
 
-        // If Wayland in use don't grant X11
 #if PLATFORM(WAYLAND) && USE(EGL)
         if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::Wayland) {
             bindWayland(sandboxArgs);
             sandboxArgs.append("--unshare-ipc");
-        } else
+        }
 #endif
+#if PLATFORM(X11)
+        if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::X11)
             bindX11(sandboxArgs);
+#endif
 
         for (const auto& pathAndPermission : launchOptions.extraWebProcessSandboxPaths) {
             sandboxArgs.appendVector(Vector<CString>({
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to