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 <peter.hutterer@who-t.net>
This commit is contained in:
parent
f69280ddcd
commit
412777664a
|
@ -3772,7 +3772,9 @@ ProcEstablishConnection(ClientPtr client)
|
||||||
|
|
||||||
prefix = (xConnClientPrefix *) ((char *) stuff + sz_xReq);
|
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->nbytesAuthProto) +
|
||||||
pad_to_int32(prefix->nbytesAuthString))
|
pad_to_int32(prefix->nbytesAuthString))
|
||||||
reason = "Bad length";
|
reason = "Bad length";
|
||||||
|
|
|
@ -646,6 +646,7 @@ typedef enum {
|
||||||
FLAG_MAX_CLIENTS,
|
FLAG_MAX_CLIENTS,
|
||||||
FLAG_IGLX,
|
FLAG_IGLX,
|
||||||
FLAG_DEBUG,
|
FLAG_DEBUG,
|
||||||
|
FLAG_ALLOW_BYTE_SWAPPED_CLIENTS,
|
||||||
} FlagValues;
|
} FlagValues;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -705,6 +706,8 @@ static OptionInfoRec FlagOptions[] = {
|
||||||
{0}, FALSE},
|
{0}, FALSE},
|
||||||
{FLAG_DEBUG, "Debug", OPTV_STRING,
|
{FLAG_DEBUG, "Debug", OPTV_STRING,
|
||||||
{0}, FALSE},
|
{0}, FALSE},
|
||||||
|
{FLAG_ALLOW_BYTE_SWAPPED_CLIENTS, "AllowByteSwappedClients", OPTV_BOOLEAN,
|
||||||
|
{0}, FALSE},
|
||||||
{-1, NULL, OPTV_NONE,
|
{-1, NULL, OPTV_NONE,
|
||||||
{0}, FALSE},
|
{0}, FALSE},
|
||||||
};
|
};
|
||||||
|
@ -746,6 +749,11 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
|
||||||
xf86Msg(X_CONFIG, "Ignoring ABI Version\n");
|
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)) {
|
if (xf86IsOptionSet(FlagOptions, FLAG_AUTO_ADD_DEVICES)) {
|
||||||
xf86GetOptValBool(FlagOptions, FLAG_AUTO_ADD_DEVICES,
|
xf86GetOptValBool(FlagOptions, FLAG_AUTO_ADD_DEVICES,
|
||||||
&xf86Info.autoAddDevices);
|
&xf86Info.autoAddDevices);
|
||||||
|
|
|
@ -677,6 +677,8 @@ Possible values are
|
||||||
or
|
or
|
||||||
.BR sync .
|
.BR sync .
|
||||||
Unset by default.
|
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"
|
.SH "MODULE SECTION"
|
||||||
The
|
The
|
||||||
.B Module
|
.B Module
|
||||||
|
|
|
@ -17,3 +17,4 @@ have_geometry=true
|
||||||
have_fullscreen=true
|
have_fullscreen=true
|
||||||
have_host_grab=true
|
have_host_grab=true
|
||||||
have_decorate=@have_libdecor@
|
have_decorate=@have_libdecor@
|
||||||
|
have_byteswappedclients=true
|
||||||
|
|
|
@ -74,4 +74,6 @@ extern _X_EXPORT Bool bgNoneRoot;
|
||||||
extern _X_EXPORT Bool CoreDump;
|
extern _X_EXPORT Bool CoreDump;
|
||||||
extern _X_EXPORT Bool NoListenAll;
|
extern _X_EXPORT Bool NoListenAll;
|
||||||
|
|
||||||
|
extern _X_EXPORT Bool AllowByteSwappedClients;
|
||||||
|
|
||||||
#endif /* OPAQUE_H */
|
#endif /* OPAQUE_H */
|
||||||
|
|
|
@ -114,6 +114,12 @@ pattern. This is the default unless -retro or -wr is specified.
|
||||||
.B \-bs
|
.B \-bs
|
||||||
disables backing store support on all screens.
|
disables backing store support on all screens.
|
||||||
.TP 8
|
.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
|
.B \-c
|
||||||
turns off key-click.
|
turns off key-click.
|
||||||
.TP 8
|
.TP 8
|
||||||
|
|
|
@ -189,6 +189,8 @@ Bool CoreDump;
|
||||||
|
|
||||||
Bool enableIndirectGLX = FALSE;
|
Bool enableIndirectGLX = FALSE;
|
||||||
|
|
||||||
|
Bool AllowByteSwappedClients = FALSE;
|
||||||
|
|
||||||
#ifdef PANORAMIX
|
#ifdef PANORAMIX
|
||||||
Bool PanoramiXExtensionDisabledHack = FALSE;
|
Bool PanoramiXExtensionDisabledHack = FALSE;
|
||||||
#endif
|
#endif
|
||||||
|
@ -523,6 +525,8 @@ UseMsg(void)
|
||||||
ErrorF("-br create root window with black background\n");
|
ErrorF("-br create root window with black background\n");
|
||||||
ErrorF("+bs enable any backing store support\n");
|
ErrorF("+bs enable any backing store support\n");
|
||||||
ErrorF("-bs disable 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 turns off key-click\n");
|
||||||
ErrorF("c # key-click volume (0-100)\n");
|
ErrorF("c # key-click volume (0-100)\n");
|
||||||
ErrorF("-cc int default color visual class\n");
|
ErrorF("-cc int default color visual class\n");
|
||||||
|
@ -720,6 +724,11 @@ ProcessCommandLine(int argc, char *argv[])
|
||||||
else
|
else
|
||||||
UseMsg();
|
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], "-br") == 0); /* default */
|
||||||
else if (strcmp(argv[i], "+bs") == 0)
|
else if (strcmp(argv[i], "+bs") == 0)
|
||||||
enableBackingStore = TRUE;
|
enableBackingStore = TRUE;
|
||||||
|
|
Loading…
Reference in New Issue