1 Feb 2005 22:44
[Fwd: Icculus quake2 - security patch (fwd)]
brendan burns <brendanburns <at> comcast.net>
2005-02-01 21:44:55 GMT
2005-02-01 21:44:55 GMT
Hey folks, The attached patch for various security fixes (mainly buffer overflows) has been incorporated into CVS. If you are running a dedicated server on the wilds of the internet, its probably worth updating... --brendan
Subject: Icculus quake2 - security patch (fwd)
Date: 2005-01-31 23:59:12 GMT
---------- Forwarded message ---------- Date: Sun, 30 Jan 2005 00:58:41 +0300 From: Andrey Nazarov <skuller-vidnoe <at> yandex.ru> To: bburns <at> icculus.org Subject: Icculus quake2 - security patch Hello, First I would like to thank you for maintaining this linux quake2 port. I found the sources very useful as a starting point for porting my own quake2 client to linux. Keep up this good work! I'm sending a patch for the latest version of Icculus Quake2 that fixes some important vulnerabilities found in quake2 engine, affecting both the server and client. Some of these vulnerabilities allow remote execution of arbitrary code, others allow denial-of-service attacks, so these fixes are essential. I'm new to linux, and I'm currently running cvs/diff on my windows box, so I can't be sure if this patch applies correctly. Sorry if there is some kind of a problem. -- Best regards, Andrey Nazarov mailto:skuller-vidnoe <at> yandex.ru
Index: client/cl_parse.c
===================================================================
RCS file: /cvs/cvsroot/quake2/src/client/cl_parse.c,v
retrieving revision 1.3
diff -u -r1.3 cl_parse.c
--- client/cl_parse.c 24 Sep 2004 22:06:52 -0000 1.3
+++ client/cl_parse.c 29 Jan 2005 20:57:01 -0000
<at> <at> -397,6 +397,9 <at> <at>
strncpy(ci->cinfo, s, sizeof(ci->cinfo));
ci->cinfo[sizeof(ci->cinfo)-1] = 0;
+ // sku - avoid potentional buffer overflow vulnerability
+ s = ci->cinfo;
+
// isolate the player's name
strncpy(ci->name, s, sizeof(ci->name));
ci->name[sizeof(ci->name)-1] = 0;
<at> <at> -528,6 +531,7 <at> <at>
int i;
char *s;
char olds[MAX_QPATH];
+ int length;
i = MSG_ReadShort (&net_message);
if (i < 0 || i >= MAX_CONFIGSTRINGS)
<at> <at> -537,6 +541,12 <at> <at>
strncpy (olds, cl.configstrings[i], sizeof(olds));
olds[sizeof(olds) - 1] = 0;
+ // sku - avoid potentional buffer overflow vulnerability
+ length = strlen( s );
+ if( length > sizeof( cl.configstrings ) - sizeof( cl.configstrings[0] ) * i - 1 ) {
+ Com_Error( ERR_DROP, "CL_ParseConfigString: oversize configstring" );
+ }
+
strcpy (cl.configstrings[i], s);
// do something apropriate
Index: qcommon/cmd.c
===================================================================
RCS file: /cvs/cvsroot/quake2/src/qcommon/cmd.c,v
retrieving revision 1.2
diff -u -r1.2 cmd.c
--- qcommon/cmd.c 3 Jan 2002 05:10:14 -0000 1.2
+++ qcommon/cmd.c 29 Jan 2005 20:57:01 -0000
<at> <at> -215,8 +215,11 <at> <at>
if (text[i] == '\n')
break;
}
-
-
+
+ // sku - removed potentional buffer overflow vulnerability
+ if( i > sizeof( line ) - 1 ) {
+ i = sizeof( line ) - 1;
+ }
memcpy (line, text, i);
line[i] = 0;
<at> <at> -657,7 +660,8 <at> <at>
{
int l;
- strcpy (cmd_args, text);
+ // sku - removed potentional buffer overflow vulnerability
+ strncpy( cmd_args, text, sizeof( cmd_args ) );
// strip off any trailing whitespace
l = strlen(cmd_args) - 1;
Index: qcommon/common.c
===================================================================
RCS file: /cvs/cvsroot/quake2/src/qcommon/common.c,v
retrieving revision 1.4
diff -u -r1.4 common.c
--- qcommon/common.c 30 Mar 2002 22:48:36 -0000 1.4
+++ qcommon/common.c 29 Jan 2005 20:57:02 -0000
<at> <at> -797,7 +797,9 <at> <at>
l = 0;
do
{
- c = MSG_ReadChar (msg_read);
+ // sku - replaced MSG_ReadChar with MSG_ReadByte to avoid
+ // potentional vulnerability
+ c = MSG_ReadByte (msg_read);
if (c == -1 || c == 0)
break;
string[l] = c;
<at> <at> -817,7 +819,9 <at> <at>
l = 0;
do
{
- c = MSG_ReadChar (msg_read);
+ // sku - replaced MSG_ReadChar with MSG_ReadByte to avoid
+ // potentional vulnerability
+ c = MSG_ReadByte (msg_read);
if (c == -1 || c == 0 || c == '\n')
break;
string[l] = c;
Index: server/sv_main.c
===================================================================
RCS file: /cvs/cvsroot/quake2/src/server/sv_main.c,v
retrieving revision 1.2
diff -u -r1.2 sv_main.c
--- server/sv_main.c 22 Mar 2002 00:24:37 -0000 1.2
+++ server/sv_main.c 29 Jan 2005 20:57:03 -0000
<at> <at> -293,8 +293,9 <at> <at>
challenge = atoi(Cmd_Argv(3));
- strncpy (userinfo, Cmd_Argv(4), sizeof(userinfo)-1);
- userinfo[sizeof(userinfo) - 1] = 0;
+ // sku - reserve 32 bytes for the IP address
+ strncpy (userinfo, Cmd_Argv(4), sizeof(userinfo)-32);
+ userinfo[sizeof(userinfo) - 32] = 0;
// force the IP key/value pair so the game can filter based on ip
Info_SetValueForKey (userinfo, "ip", NET_AdrToString(net_from));
<at> <at> -317,8 +318,11 <at> <at>
{
if (NET_CompareBaseAdr (net_from, svs.challenges[i].adr))
{
- if (challenge == svs.challenges[i].challenge)
+ // sku - ignore zero challenges
+ if( svs.challenges[i].challenge && challenge == svs.challenges[i].challenge ) {
+ svs.challenges[i].challenge = 0;
break; // good
+ }
Netchan_OutOfBandPrint (NS_SERVER, adr, "print\nBad challenge.\n");
return;
}
<at> <at> -342,6 +346,11 <at> <at>
&& ( cl->netchan.qport == qport
|| adr.port == cl->netchan.remote_address.port ) )
{
+ // sku - avoid reusing slot of the client already connected
+ if( cl->state != cs_zombie ) {
+ Netchan_OutOfBandPrint( NS_SERVER, adr, "print\nConnected client from this IP is already
present.\n" );
+ return;
+ }
if (!NET_IsLocalAddress (adr) && (svs.realtime - cl->lastconnect) <
((int)sv_reconnect_limit->value * 1000))
{
Com_DPrintf ("%s:reconnect rejected : too soon\n", NET_AdrToString (adr));
Index: server/sv_user.c
===================================================================
RCS file: /cvs/cvsroot/quake2/src/server/sv_user.c,v
retrieving revision 1.2
diff -u -r1.2 sv_user.c
--- server/sv_user.c 21 Mar 2002 04:44:46 -0000 1.2
+++ server/sv_user.c 29 Jan 2005 20:57:04 -0000
<at> <at> -142,6 +142,9 <at> <at>
}
start = atoi(Cmd_Argv(2));
+ if( start < 0 ) {
+ start = 0; // sku - catch negative offsets
+ }
// write a packet full of data
<at> <at> -150,9 +153,18 <at> <at>
{
if (sv.configstrings[start][0])
{
+ int length;
+
+ // sku - write configstrings that exceed MAX_QPATH in proper-sized chunks
+ length = strlen( sv.configstrings[start] );
+ if( length > MAX_QPATH ) {
+ length = MAX_QPATH;
+ }
+
MSG_WriteByte (&sv_client->netchan.message, svc_configstring);
MSG_WriteShort (&sv_client->netchan.message, start);
- MSG_WriteString (&sv_client->netchan.message, sv.configstrings[start]);
+ SZ_Write (&sv_client->netchan.message, sv.configstrings[start], length);
+ MSG_WriteByte (&sv_client->netchan.message, 0);
}
start++;
}
<at> <at> -199,6 +211,9 <at> <at>
}
start = atoi(Cmd_Argv(2));
+ if( start < 0 ) {
+ start = 0;
+ }
memset (&nullstate, 0, sizeof(nullstate));
<at> <at> -398,7 +413,7 <at> <at>
*/
void SV_ShowServerinfo_f (void)
{
- Info_Print (Cvar_Serverinfo());
+// Info_Print (Cvar_Serverinfo());
}
RSS Feed