mirror of
https://github.com/X11Libre/xserver.git
synced 2026-10-21 01:13:38 +00:00
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: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2263> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This commit is contained in:
parent
21f82727a4
commit
7770acd009
9 changed files with 316 additions and 4 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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 */
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -77,6 +77,7 @@ typedef struct {
|
|||
* connections */
|
||||
Bool pmFlag;
|
||||
MessageType iglxFrom;
|
||||
MessageType fontserverFrom;
|
||||
XF86_GlxVisuals glxVisuals;
|
||||
MessageType glxVisualsFrom;
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ OR PERFORMANCE OF THIS SOFTWARE.
|
|||
*/
|
||||
|
||||
#include <dix-config.h>
|
||||
#include <stdbool.h>
|
||||
|
||||
#ifdef __CYGWIN__
|
||||
#include <stdlib.h>
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
"<HHH BB HH",
|
||||
0, # status = AuthSuccess
|
||||
2, # major_version = FS_PROTOCOL
|
||||
0, # minor_version = FS_PROTOCOL_MINOR
|
||||
0, # num_alternates
|
||||
0, # auth_index
|
||||
0, # alternate_len
|
||||
0, # auth_len
|
||||
)
|
||||
accept = struct.pack(
|
||||
"<IHHI",
|
||||
3, # length in 4-byte words
|
||||
8192, # max_request_len
|
||||
0, # vendor_len
|
||||
1, # release_number
|
||||
)
|
||||
client.sendall(setup + accept)
|
||||
|
||||
while True:
|
||||
if not client.recv(4096):
|
||||
break
|
||||
except OSError:
|
||||
pass
|
||||
finally:
|
||||
try:
|
||||
client.close()
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
def stop(self):
|
||||
for c in self._clients:
|
||||
try:
|
||||
c.close()
|
||||
except OSError:
|
||||
pass
|
||||
for s in self._socks:
|
||||
try:
|
||||
s.close()
|
||||
except OSError:
|
||||
pass
|
||||
for t in self._threads:
|
||||
t.join(timeout=2)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fake_font_server():
|
||||
"""Start a fake font server that speaks minimal xfs protocol."""
|
||||
fs = _FakeFontServer()
|
||||
fs.start()
|
||||
yield fs
|
||||
fs.stop()
|
||||
|
||||
|
||||
class TestFontAliasOverflow:
|
||||
|
|
@ -79,7 +218,7 @@ class TestFontAliasOverflow:
|
|||
# and copies the oversized target into the stack buffer.
|
||||
req = x11.ListFontsRequest(pattern=alias_name, max_names=10)
|
||||
xclient.send_request(req.to_bytes())
|
||||
time.sleep(0.5)
|
||||
xclient.flush_responses(timeout=1.0)
|
||||
|
||||
assert xserver.is_alive, (
|
||||
"Server crashed - font alias stack buffer overflow (ZDI-CAN-30136)"
|
||||
|
|
@ -89,3 +228,115 @@ class TestFontAliasOverflow:
|
|||
req = x11.SetFontPathRequest(paths=original_paths)
|
||||
xclient.send_request(req.to_bytes())
|
||||
xclient.flush_responses(timeout=1.0)
|
||||
|
||||
|
||||
class TestFontServerConnections:
|
||||
"""Tests for the +/-fontserverconnections option."""
|
||||
|
||||
@pytest.fixture
|
||||
def fs_entry(self, fake_font_server):
|
||||
return f"tcp/localhost:{fake_font_server.port}"
|
||||
|
||||
@pytest.fixture(
|
||||
params=[
|
||||
pytest.param("+fontserverconnections", id="enabled"),
|
||||
pytest.param("-fontserverconnections", id="disabled"),
|
||||
pytest.param("", id="default"),
|
||||
]
|
||||
)
|
||||
def xserver_args(self, request, fs_entry):
|
||||
args = ["-fp", f"built-ins,{fs_entry}"]
|
||||
if request.param:
|
||||
args.insert(0, request.param)
|
||||
return args
|
||||
|
||||
def test_fontserver_in_default_fp(self, xserver, xclient, fs_entry):
|
||||
"""
|
||||
When a font server entry is passed via -fp on the command line,
|
||||
it should be silently dropped from the font path at startup
|
||||
when font server connections are disabled.
|
||||
|
||||
Our xserver fixture will start with the fontserver path added.
|
||||
|
||||
Note: when +fontserverconnections is set, the font server
|
||||
entry may still be absent if libXfont2 was built without
|
||||
font server client support (--disable-fc, as done by Debian).
|
||||
In that case no FPE recognizes the ``tcp/...`` path entry and
|
||||
it is silently dropped regardless of our setting. We only
|
||||
assert that our guard does not *prevent* it.
|
||||
"""
|
||||
enabled = "+fontserverconnections" in xserver.extra_args
|
||||
|
||||
req = x11.GetFontPathRequest()
|
||||
xclient.send_request(req.to_bytes())
|
||||
resp = xclient.recv_response(timeout=3.0)
|
||||
assert isinstance(resp, X11Reply), "GetFontPath failed"
|
||||
paths = x11.GetFontPathReply.from_reply(resp.data).paths
|
||||
|
||||
assert "built-ins" in paths, "built-ins should remain in the font path"
|
||||
|
||||
if not enabled:
|
||||
assert fs_entry not in paths, (
|
||||
"Font server entry should have been stripped from "
|
||||
"the default font path when font server "
|
||||
"connections are disabled"
|
||||
)
|
||||
|
||||
assert xserver.is_alive, "Server crashed during font path setup"
|
||||
|
||||
def test_set_fontpath_with_fontserver(self, xserver, xclient, fs_entry):
|
||||
"""
|
||||
SetFontPath with a font server entry should fail with BadAlloc when
|
||||
font server connections are disabled (the default or explicit
|
||||
-fontserverconnections).
|
||||
|
||||
When +fontserverconnections is set it should succeed but
|
||||
where libXfont is built with --disable-fc (e.g. Debian)
|
||||
it will fail with BadValue (not BadAlloc).
|
||||
"""
|
||||
enabled = "+fontserverconnections" in xserver.extra_args
|
||||
|
||||
# Get the original font path first
|
||||
req = x11.GetFontPathRequest()
|
||||
xclient.send_request(req.to_bytes())
|
||||
resp = xclient.recv_response(timeout=3.0)
|
||||
assert isinstance(resp, X11Reply), "GetFontPath failed"
|
||||
original_paths = x11.GetFontPathReply.from_reply(resp.data).paths
|
||||
|
||||
# Try to set font path to include a font server entry
|
||||
# alongside the existing paths so the default font
|
||||
# remains available.
|
||||
new_paths = original_paths + [fs_entry]
|
||||
req = x11.SetFontPathRequest(paths=new_paths)
|
||||
xclient.send_request(req.to_bytes())
|
||||
resp = xclient.recv_response(timeout=3.0)
|
||||
|
||||
if enabled:
|
||||
# With +fontserverconnections we may get BadValue
|
||||
# if libXfont2 was built with --disable-fc. As
|
||||
# long as it's not BadAlloc we assume it worked.
|
||||
if isinstance(resp, X11Error):
|
||||
assert resp.error_code != x11.BadAlloc, (
|
||||
"SetFontPath returned BadAlloc -- "
|
||||
"_init_fs_handlers guard fired despite "
|
||||
"+fontserverconnections being set"
|
||||
)
|
||||
else:
|
||||
assert isinstance(resp, X11Error), (
|
||||
"Expected SetFontPath to fail with a font server "
|
||||
f"entry when connections are disabled, got {resp}"
|
||||
)
|
||||
|
||||
# Verify the font path is unchanged
|
||||
req = x11.GetFontPathRequest()
|
||||
xclient.send_request(req.to_bytes())
|
||||
resp = xclient.recv_response(timeout=3.0)
|
||||
assert isinstance(resp, X11Reply), (
|
||||
"GetFontPath failed after rejected SetFontPath"
|
||||
)
|
||||
current_paths = x11.GetFontPathReply.from_reply(resp.data).paths
|
||||
assert current_paths == original_paths, (
|
||||
"Font path was modified despite SetFontPath returning an error"
|
||||
)
|
||||
|
||||
assert xserver.is_alive, "Server crashed handling font server path"
|
||||
|
|
|
|||
Loading…
Reference in a new issue