Allow disabling byte-swapped clients

The X server swapping code is a huge attack surface, much of this code
is untested and prone to security issues. The use-case of byte-swapped
clients is very niche, so allow users to disable this if they don't
need it, using either a config option or commandline flag.

For Xorg, this adds the ServerFlag "AllowByteSwappedClients" "off".
For all DDX, this adds the commandline options +byteswappedclients and
-byteswappedclients to enable or disable, respectively.

Fixes #1201
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
(cherry picked from commit 412777664a20dd3561b936c02c96571a756fe9b2)
(cherry picked from commit af5cd5acc9012e527ee869f8e98bf6c2e9a02ca4)
Backport to server-21.1-branch modified to keep byte-swapping enabled
by default but easy to disable by users or admins (or even by distros
shipping an xorg.conf.d fragment in their packages).

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1440>

6 files changed, 37 insertions, 3 deletions

diff --git a/dix/dispatch.c b/dix/dispatch.c
index 460296197..8f7452d87 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3780,9 +3780,11 @@ ProcEstablishConnection(ClientPtr client)
     auth_proto = (char *) prefix + sz_xConnClientPrefix;
     auth_string = auth_proto + pad_to_int32(prefix->nbytesAuthProto);
 
-    if ((client->req_len << 2) != sz_xReq + sz_xConnClientPrefix +
-	pad_to_int32(prefix->nbytesAuthProto) +
-	pad_to_int32(prefix->nbytesAuthString))
+    if (client->swapped && !AllowByteSwappedClients) {
+        reason = "Prohibited client endianess, see the Xserver man page ";
+    } else if ((client->req_len << 2) != sz_xReq + sz_xConnClientPrefix +
+               pad_to_int32(prefix->nbytesAuthProto) +
+               pad_to_int32(prefix->nbytesAuthString))
         reason = "Bad length";
     else if ((prefix->majorVersion != X_PROTOCOL) ||
         (prefix->minorVersion != X_PROTOCOL_REVISION))
diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
index 5d814c148..8d8ebf1b5 100644
--- a/hw/xfree86/common/xf86Config.c
+++ b/hw/xfree86/common/xf86Config.c
@@ -646,6 +646,7 @@ typedef enum {
     FLAG_MAX_CLIENTS,
     FLAG_IGLX,
     FLAG_DEBUG,
+    FLAG_ALLOW_BYTE_SWAPPED_CLIENTS,
 } FlagValues;
 
 /**
@@ -705,6 +706,8 @@ static OptionInfoRec FlagOptions[] = {
      {0}, FALSE},
     {FLAG_DEBUG, "Debug", OPTV_STRING,
      {0}, FALSE},
+    {FLAG_ALLOW_BYTE_SWAPPED_CLIENTS, "AllowByteSwappedClients", OPTV_BOOLEAN,
+     {0}, FALSE},
     {-1, NULL, OPTV_NONE,
      {0}, FALSE},
 };
@@ -746,6 +749,14 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
         xf86Msg(X_CONFIG, "Ignoring ABI Version\n");
     }
 
+    xf86GetOptValBool(FlagOptions, FLAG_ALLOW_BYTE_SWAPPED_CLIENTS, &AllowByteSwappedClients);
+    if (AllowByteSwappedClients) {
+        xf86Msg(X_CONFIG, "Allowing byte-swapped clients\n");
+    }
+    else {
+        xf86Msg(X_CONFIG, "Prohibiting byte-swapped clients\n");
+    }
+
     if (xf86IsOptionSet(FlagOptions, FLAG_AUTO_ADD_DEVICES)) {
         xf86GetOptValBool(FlagOptions, FLAG_AUTO_ADD_DEVICES,
                           &xf86Info.autoAddDevices);
diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index ac88d7e7a..ed125b3ee 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -677,6 +677,9 @@ Possible values are
 or
 .BR sync .
 Unset by default.
+.TP 7
+.BI "Option \*qAllowByteSwappedClients\*q  \*q" boolean \*q
+Allow clients with a different byte-order than the server. Enabled by default.
 .SH "MODULE SECTION"
 The
 .B Module
diff --git a/include/opaque.h b/include/opaque.h
index 256261c2a..398d4b4e5 100644
--- a/include/opaque.h
+++ b/include/opaque.h
@@ -74,4 +74,6 @@ extern _X_EXPORT Bool bgNoneRoot;
 extern _X_EXPORT Bool CoreDump;
 extern _X_EXPORT Bool NoListenAll;
 
+extern _X_EXPORT Bool AllowByteSwappedClients;
+
 #endif                          /* OPAQUE_H */
diff --git a/man/Xserver.man b/man/Xserver.man
index 764bd1d90..d6e18ffdd 100644
--- a/man/Xserver.man
+++ b/man/Xserver.man
@@ -114,6 +114,13 @@ pattern.   This is the default unless -retro or -wr is specified.
 .B \-bs
 disables backing store support on all screens.
 .TP 8
+.B \+byteswappedclients
+Allow connections from clients with an endianess different to that of the server.
+This is the default unless \fB\-byteswappedclients\fP is specified.
+.TP 8
+.B \-byteswappedclients
+Prohibit connections from clients with an endianess different to that of the server.
+.TP 8
 .B \-c
 turns off key-click.
 .TP 8
diff --git a/os/utils.c b/os/utils.c
index 92a66e81a..f0d022fba 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -189,6 +189,8 @@ Bool CoreDump;
 
 Bool enableIndirectGLX = FALSE;
 
+Bool AllowByteSwappedClients = TRUE;
+
 #ifdef PANORAMIX
 Bool PanoramiXExtensionDisabledHack = FALSE;
 #endif
@@ -523,6 +525,8 @@ UseMsg(void)
     ErrorF("-br                    create root window with black background\n");
     ErrorF("+bs                    enable any backing store support\n");
     ErrorF("-bs                    disable any backing store support\n");
+    ErrorF("+byteswappedclients    Allow clients with endianess different to that of the server\n");
+    ErrorF("-byteswappedclients    Prohibit clients with endianess different to that of the server\n");
     ErrorF("-c                     turns off key-click\n");
     ErrorF("c #                    key-click volume (0-100)\n");
     ErrorF("-cc int                default color visual class\n");
@@ -719,6 +723,11 @@ ProcessCommandLine(int argc, char *argv[])
             else
                 UseMsg();
         }
+        else if (strcmp(argv[i], "-byteswappedclients") == 0) {
+            AllowByteSwappedClients = FALSE;
+        } else if (strcmp(argv[i], "+byteswappedclients") == 0) {
+            AllowByteSwappedClients = TRUE;
+        }
         else if (strcmp(argv[i], "-br") == 0);  /* default */
         else if (strcmp(argv[i], "+bs") == 0)
             enableBackingStore = TRUE;