From 7770acd00923575c3ad2b964fd47324d487ae1ba Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 21 Jul 2026 11:25:40 +1000 Subject: [PATCH] Disable font server connections by default At this point, allowing X Font Server connections just introduces us to possible security issues since the code effectively trusts any font server and few checks are in place. Since font servers also haven't been used for ages, let's disable them by default. Use-cases that require them will have to pass +fontserverconnections on the commandline and/or set the corresponding ServerFlags option. The tests are a bit hacky: Debian builds libXfont2 with --disable-fc so we need to cater for the case where the X server enables fontserver support but libXfont then blocks it and we still need to treat this as success. Assisted-by: Claude:claude-opus-4-6 Part-of: Signed-off-by: Enrico Weigelt, metux IT consult --- dix/dix_priv.h | 3 + dix/dixfonts.c | 7 + hw/xfree86/common/xf86Config.c | 21 +++ hw/xfree86/common/xf86Init.c | 4 + hw/xfree86/man/xorg.conf.man | 9 ++ include/xf86Privstr.h | 1 + man/Xserver.man | 8 + os/utils.c | 8 + test/pyxtest/test_font.py | 259 ++++++++++++++++++++++++++++++++- 9 files changed, 316 insertions(+), 4 deletions(-) diff --git a/dix/dix_priv.h b/dix/dix_priv.h index 67cd3da2d..67a645d4b 100644 --- a/dix/dix_priv.h +++ b/dix/dix_priv.h @@ -65,6 +65,9 @@ extern Bool party_like_its_1989; /* needed by libglx and libglamor (server modules) */ extern _X_EXPORT Bool enableIndirectGLX; +/* whether font server connections are allowed in the font path */ +extern bool enableFontServerConnections; + /* length of ConnectionInfo block, MUST be multiple of 4 */ extern size_t ConnectionInfoSize; extern size_t dixConnBlockScreenStart(const char *connInfo); diff --git a/dix/dixfonts.c b/dix/dixfonts.c index f2b9ab08e..e113d5656 100644 --- a/dix/dixfonts.c +++ b/dix/dixfonts.c @@ -2011,6 +2011,13 @@ adjust_fs_wait_for_delay(void *wt, unsigned long newdelay) static int _init_fs_handlers(FontPathElementPtr fpe, FontBlockHandlerProcPtr block_handler) { + if (!enableFontServerConnections) { + LogMessage(X_WARNING, + "Ignoring font server \"%s\", font server connections " + "are disabled.\n", fpe->name); + return AllocError; + } + /* if server has reset, make sure the b&w handlers are reinstalled */ if (last_server_gen < serverGeneration) { last_server_gen = serverGeneration; diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c index 0a5be5f2c..f51b53c7e 100644 --- a/hw/xfree86/common/xf86Config.c +++ b/hw/xfree86/common/xf86Config.c @@ -229,6 +229,17 @@ xf86ValidateFontPath(char *path) * Either an OK directory, or a font server name. So add it to * the path. */ + if (*path_elem != '/' && + strcmp(path_elem, "built-ins") != 0 && + strncmp(path_elem, "catalogue:", strlen("catalogue:")) != 0 && + !enableFontServerConnections) { + xf86Msg(X_WARNING, + "Ignoring font server entry \"%s\" in font path.\n", + path_elem); + xf86ErrorF("\tFont server connections are disabled.\n"); + xf86ErrorF("\tUse +fontserverconnections to allow them.\n"); + continue; + } if (out_pnt != tmp_path) *out_pnt++ = ','; strcat(out_pnt, path_elem); @@ -656,6 +667,7 @@ typedef enum { FLAG_DEBUG, FLAG_ALLOW_BYTE_SWAPPED_CLIENTS, FLAG_SINGLE_DRIVER, + FLAG_FONTSERVER, } FlagValues; /** @@ -721,6 +733,8 @@ static OptionInfoRec FlagOptions[] = { {0}, FALSE}, {FLAG_SINGLE_DRIVER, "SingleDriver", OPTV_BOOLEAN, {0}, FALSE}, + {FLAG_FONTSERVER, "FontServerConnections", OPTV_BOOLEAN, + {0}, FALSE}, {-1, NULL, OPTV_NONE, {0}, FALSE}, }; @@ -905,6 +919,13 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts) } #endif + if (xf86Info.fontserverFrom != X_CMDLINE) { + if (xf86GetOptValBool(FlagOptions, FLAG_FONTSERVER, &value)) { + enableFontServerConnections = !!value; + xf86Info.fontserverFrom = X_CONFIG; + } + } + xf86Info.debug = xf86GetOptValString(FlagOptions, FLAG_DEBUG); /* if we're not hotplugging, force some input devices to exist */ diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index 854568abf..0ba13ddf6 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -1159,6 +1159,10 @@ ddxProcessArgument(int argc, char **argv, int i) xf86Info.ShareVTs = TRUE; return 1; } + if (!strcmp(argv[i], "-fontserverconnections") || !strcmp(argv[i], "+fontserverconnections")) { + xf86Info.fontserverFrom = X_CMDLINE; + return 0; + } if (!strcmp(argv[i], "-iglx") || !strcmp(argv[i], "+iglx")) { xf86Info.iglxFrom = X_CMDLINE; return 0; diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man index a7587348f..5e95fec79 100644 --- a/hw/xfree86/man/xorg.conf.man +++ b/hw/xfree86/man/xorg.conf.man @@ -652,6 +652,15 @@ Include the default font path even if other paths are specified in xorg.conf. If enabled, other font paths are included as well. Enabled by default. .TP 7 +.BI "Option \*qFontServerConnections\*q \*q" boolean \*q +Allow connections to font servers (e.g.\& +.BR tcp/fontserver:7100 ) +in the font path. Font servers are rarely used and are a source of +security vulnerabilities. Disabled by default. +Can also be set with the +.B +fontserverconnections +command line option. +.TP 7 .BI "Option \*qIgnoreABI\*q \*q" boolean \*q Allow modules built for a different, potentially incompatible version of the X server to load. Disabled by default. diff --git a/include/xf86Privstr.h b/include/xf86Privstr.h index 719fb7e4f..d123978b5 100644 --- a/include/xf86Privstr.h +++ b/include/xf86Privstr.h @@ -77,6 +77,7 @@ typedef struct { * connections */ Bool pmFlag; MessageType iglxFrom; + MessageType fontserverFrom; XF86_GlxVisuals glxVisuals; MessageType glxVisualsFrom; diff --git a/man/Xserver.man b/man/Xserver.man index c4e1fb3c6..2e7698c4c 100644 --- a/man/Xserver.man +++ b/man/Xserver.man @@ -194,6 +194,14 @@ of directories which the X server searches for font databases. See the FONTS section of this manual page for more information and the default list. .TP 8 +.B \-fontserverconnections +Prohibit font server connections in the font path. Font servers are +rarely used and are a source of security vulnerabilities. +This is the default unless +fontserverconnections is specified. +.TP 8 +.B +fontserverconnections +Allow font server connections in the font path. +.TP 8 .B \-help prints a usage message. .TP 8 diff --git a/os/utils.c b/os/utils.c index c93f75544..39b32b77b 100644 --- a/os/utils.c +++ b/os/utils.c @@ -49,6 +49,7 @@ OR PERFORMANCE OF THIS SOFTWARE. */ #include +#include #ifdef __CYGWIN__ #include @@ -136,6 +137,7 @@ __stdcall unsigned long GetTickCount(void); Bool CoreDump; Bool enableIndirectGLX = FALSE; +bool enableFontServerConnections = false; #ifdef XINERAMA Bool PanoramiXExtensionDisabledHack = FALSE; @@ -288,6 +290,8 @@ UseMsg(void) ErrorF("-f # bell base (0-100)\n"); ErrorF("-fakescreenfps # fake screen default fps (1-600)\n"); ErrorF("-fp string default font path\n"); + ErrorF("+fontserverconnections Allow font server connections in the font path\n"); + ErrorF("-fontserverconnections Prohibit font server connections in the font path (default)\n"); ErrorF("-help prints message with these options\n"); ErrorF("+iglx Allow creating indirect GLX contexts\n"); ErrorF("-iglx Prohibit creating indirect GLX contexts (default)\n"); @@ -553,6 +557,10 @@ ProcessCommandLine(int argc, char *argv[]) UseMsg(); exit(0); } + else if (strcmp(argv[i], "+fontserverconnections") == 0) + enableFontServerConnections = true; + else if (strcmp(argv[i], "-fontserverconnections") == 0) + enableFontServerConnections = false; else if (strcmp(argv[i], "+iglx") == 0) enableIndirectGLX = TRUE; else if (strcmp(argv[i], "-iglx") == 0) diff --git a/test/pyxtest/test_font.py b/test/pyxtest/test_font.py index 14a15107f..d687233ba 100644 --- a/test/pyxtest/test_font.py +++ b/test/pyxtest/test_font.py @@ -1,13 +1,152 @@ # SPDX-License-Identifier: MIT # -# Security tests for font alias handling vulnerabilities. +# Security tests for font alias handling vulnerabilities and +# tests for the font server connections option. import os -import time +import socket +import struct +import threading import pytest from proto import x11 -from xclient import X11Reply +from xclient import X11Error, X11Reply + + +class _FakeFontServer: + """Minimal font server that completes the xfs protocol handshake. + + Listens on a random TCP port and speaks just enough of the xfs + protocol for libXfont2's fserve FPE to successfully initialize. + The handshake consists of: + 1. Read fsConnClientPrefix (8 bytes) from the client + 2. Send fsConnSetup (12 bytes) with status=AuthSuccess + 3. Send fsConnSetupAccept (12 bytes) + 4. Keep the connection open (the client may send further requests + like SetResolution which we silently ignore) + + Note: this only works when libXfont2 is built with font server + support (--enable-fc, the upstream default). Some distributions + (e.g. Debian) build libXfont2 with --disable-fc, in which case + the fserve FPE is not registered and the entry will be rejected + with BadValue regardless of the +fontserverconnections setting. + """ + + def __init__(self): + self._socks = [] + self._threads = [] + self._clients = [] + self._lock = threading.Lock() + + self.port = None + for family, bind_addr, opts in [ + (socket.AF_INET, "0.0.0.0", []), + (socket.AF_INET6, "::", [(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, 1)]), + ]: + try: + sock = socket.socket(family, socket.SOCK_STREAM) + except OSError: + continue + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + for opt in opts: + sock.setsockopt(*opt) + + bind_port = self.port if self.port is not None else 0 + try: + sock.bind((bind_addr, bind_port)) + except OSError: + sock.close() + continue + sock.listen(5) + + if self.port is None: + self.port = sock.getsockname()[1] + + self._socks.append(sock) + + if not self._socks: + raise RuntimeError("Could not bind fake font server to any address") + + def start(self): + for sock in self._socks: + t = threading.Thread(target=self._serve, args=(sock,), daemon=True) + t.start() + self._threads.append(t) + + def _serve(self, sock): + while True: + try: + client, _ = sock.accept() + except OSError: + break + with self._lock: + self._clients.append(client) + threading.Thread( + target=self._handle_client, args=(client,), daemon=True + ).start() + + def _handle_client(self, client): + client.settimeout(10) + try: + data = b"" + while len(data) < 8: + chunk = client.recv(8 - len(data)) + if not chunk: + return + data += chunk + + setup = struct.pack( + "