From e6bf129b81251bcd2a6e2f8e5c77a8d238a043bf Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 13 Jan 2023 15:23:38 +1000 Subject: [PATCH] Disallow byte-swapped clients (#2159489) --- ...llow-byte-swapped-clients-by-default.patch | 272 ++++++++++++++++++ xorg-x11-server.spec | 11 +- 2 files changed, 282 insertions(+), 1 deletion(-) create mode 100644 0001-Disallow-byte-swapped-clients-by-default.patch diff --git a/0001-Disallow-byte-swapped-clients-by-default.patch b/0001-Disallow-byte-swapped-clients-by-default.patch new file mode 100644 index 0000000..2cbf798 --- /dev/null +++ b/0001-Disallow-byte-swapped-clients-by-default.patch @@ -0,0 +1,272 @@ +From 73d6e888c6058b28a0e87ab65aa4172b17d8327d Mon Sep 17 00:00:00 2001 +From: Peter Hutterer +Date: Mon, 19 Dec 2022 10:34:29 +1000 +Subject: [PATCH xserver] Fix some indentation issues + +Signed-off-by: Peter Hutterer +--- + dix/dispatch.c | 22 +++++++++++----------- + 1 file changed, 11 insertions(+), 11 deletions(-) + +diff --git a/dix/dispatch.c b/dix/dispatch.c +index 210df75c63..e38a8fecaa 100644 +--- a/dix/dispatch.c ++++ b/dix/dispatch.c +@@ -492,10 +492,10 @@ Dispatch(void) + if (!WaitForSomething(clients_are_ready())) + continue; + +- /***************** +- * Handle events in round robin fashion, doing input between +- * each round +- *****************/ ++ /***************** ++ * Handle events in round robin fashion, doing input between ++ * each round ++ *****************/ + + if (!dispatchException && clients_are_ready()) { + client = SmartScheduleClient(); +@@ -3657,11 +3657,11 @@ ProcInitialConnection(ClientPtr client) + prefix = (xConnClientPrefix *) ((char *)stuff + sz_xReq); + order = prefix->byteOrder; + if (order != 'l' && order != 'B' && order != 'r' && order != 'R') +- return client->noClientException = -1; ++ return client->noClientException = -1; + if (((*(char *) &whichbyte) && (order == 'B' || order == 'R')) || +- (!(*(char *) &whichbyte) && (order == 'l' || order == 'r'))) { +- client->swapped = TRUE; +- SwapConnClientPrefix(prefix); ++ (!(*(char *) &whichbyte) && (order == 'l' || order == 'r'))) { ++ client->swapped = TRUE; ++ SwapConnClientPrefix(prefix); + } + stuff->reqType = 2; + stuff->length += bytes_to_int32(prefix->nbytesAuthProto) + +@@ -3670,7 +3670,7 @@ ProcInitialConnection(ClientPtr client) + swaps(&stuff->length); + } + if (order == 'r' || order == 'R') { +- client->local = FALSE; ++ client->local = FALSE; + } + ResetCurrentRequest(client); + return Success; +@@ -3781,8 +3781,8 @@ ProcEstablishConnection(ClientPtr client) + 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)) ++ pad_to_int32(prefix->nbytesAuthProto) + ++ pad_to_int32(prefix->nbytesAuthString)) + reason = "Bad length"; + else if ((prefix->majorVersion != X_PROTOCOL) || + (prefix->minorVersion != X_PROTOCOL_REVISION)) +-- +2.39.0 + +From f69280ddcdd3115ee4717f22e85e0f43569b60dd Mon Sep 17 00:00:00 2001 +From: Peter Hutterer +Date: Tue, 20 Dec 2022 11:40:16 +1000 +Subject: [PATCH xserver] dix: localize two variables + +Signed-off-by: Peter Hutterer +--- + dix/dispatch.c | 8 ++++---- + 1 file changed, 4 insertions(+), 4 deletions(-) + +diff --git a/dix/dispatch.c b/dix/dispatch.c +index c651c3d887..92be773e6c 100644 +--- a/dix/dispatch.c ++++ b/dix/dispatch.c +@@ -3766,14 +3766,11 @@ int + ProcEstablishConnection(ClientPtr client) + { + const char *reason; +- char *auth_proto, *auth_string; + xConnClientPrefix *prefix; + + REQUEST(xReq); + + prefix = (xConnClientPrefix *) ((char *) stuff + sz_xReq); +- 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) + +@@ -3782,12 +3779,15 @@ ProcEstablishConnection(ClientPtr client) + else if ((prefix->majorVersion != X_PROTOCOL) || + (prefix->minorVersion != X_PROTOCOL_REVISION)) + reason = "Protocol version mismatch"; +- else ++ else { ++ char *auth_proto = (char *) prefix + sz_xConnClientPrefix; ++ char *auth_string = auth_proto + pad_to_int32(prefix->nbytesAuthProto); + reason = ClientAuthorized(client, + (unsigned short) prefix->nbytesAuthProto, + auth_proto, + (unsigned short) prefix->nbytesAuthString, + auth_string); ++ } + + return (SendConnSetup(client, reason)); + } +-- +2.39.0 + +From 412777664a20dd3561b936c02c96571a756fe9b2 Mon Sep 17 00:00:00 2001 +From: Peter Hutterer +Date: Tue, 20 Dec 2022 10:42:03 +1000 +Subject: [PATCH xserver] Disallow byte-swapped clients by default + +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 let's disable this by default and allow it +only when the respective config option or commandline flag is given. + +For Xorg, this adds the ServerFlag "AllowByteSwappedClients" "on". +For all DDX, this adds the commandline options +byteswappedclients and +-byteswappedclients to enable or disable, respectively. + +Fixes #1201 + +https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1029 + +Signed-off-by: Peter Hutterer +--- + dix/dispatch.c | 4 +++- + hw/xfree86/common/xf86Config.c | 8 ++++++++ + hw/xfree86/man/xorg.conf.man | 2 ++ + hw/xwayland/xwayland.pc.in | 1 + + include/opaque.h | 2 ++ + man/Xserver.man | 6 ++++++ + os/utils.c | 9 +++++++++ + 7 files changed, 31 insertions(+), 1 deletion(-) + +diff --git a/dix/dispatch.c b/dix/dispatch.c +index 92be773e6c..9c26753a96 100644 +--- a/dix/dispatch.c ++++ b/dix/dispatch.c +@@ -3772,7 +3772,9 @@ ProcEstablishConnection(ClientPtr client) + + prefix = (xConnClientPrefix *) ((char *) stuff + sz_xReq); + +- if ((client->req_len << 2) != sz_xReq + sz_xConnClientPrefix + ++ 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"; +diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c +index 5d814c1485..41acb25aa2 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,11 @@ 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"); ++ } ++ + 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 01b47247ee..d057f26ecd 100644 +--- a/hw/xfree86/man/xorg.conf.man ++++ b/hw/xfree86/man/xorg.conf.man +@@ -677,6 +677,8 @@ Possible values are + or + .BR sync . + Unset by default. ++.BI "Option \*qAllowByteSwappedClients\*q \*q" boolean \*q ++Allow clients with a different byte-order than the server. Disabled by default. + .SH "MODULE SECTION" + The + .B Module +diff --git a/include/opaque.h b/include/opaque.h +index 256261c2ad..398d4b4e51 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 764bd1d907..e7adf9eb35 100644 +--- a/man/Xserver.man ++++ b/man/Xserver.man +@@ -114,6 +114,12 @@ 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. ++.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 fe94912f34..405bf7d8b4 100644 +--- a/os/utils.c ++++ b/os/utils.c +@@ -189,6 +189,8 @@ Bool CoreDump; + + Bool enableIndirectGLX = FALSE; + ++Bool AllowByteSwappedClients = FALSE; ++ + #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"); +@@ -720,6 +724,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; +-- +2.39.0 + diff --git a/xorg-x11-server.spec b/xorg-x11-server.spec index a79dac2..3fd8caf 100644 --- a/xorg-x11-server.spec +++ b/xorg-x11-server.spec @@ -46,7 +46,7 @@ Summary: X.Org X11 X server Name: xorg-x11-server Version: 1.20.14 -Release: 13%{?gitdate:.%{gitdate}}%{?dist} +Release: 14%{?gitdate:.%{gitdate}}%{?dist} URL: http://www.x.org License: MIT @@ -135,6 +135,12 @@ Patch121: 0007-xkb-reset-the-radio_groups-pointer-to-NULL-after-fre.patch # Fix for buggy patch to CVE-2022-46340 Patch122: 0008-Xext-fix-invalid-event-type-mask-in-XTestSwapFakeInp.patch +# Only on F38 and later +%if 0%{fedora} >= 38 +# Upstream commits 73d6e88, f69280dd and 4127776, minus the xwayland.pc.in change +Patch200: 0001-Disallow-byte-swapped-clients-by-default.patch +%endif + BuildRequires: make BuildRequires: systemtap-sdt-devel BuildRequires: git-core @@ -548,6 +554,9 @@ find %{inst_srcdir}/hw/xfree86 -name \*.c -delete %changelog +* Fri Jan 13 2023 Peter Hutterer - 1.20.14-14 +- Disallow byte-swapped clients (#2159489) + * Wed Jan 11 2023 Olivier Fourdan - 1.20.14-13 - Rename boolean config value field from bool to boolean to fix drivers build failures due to a conflict with C++ and stdbool.h