--- Begin Message ---
- To: Debian Bug Tracking System <submit@bugs.debian.org>
- Subject: pu: package tremulous/1.1.0-7~squeeze1 (contrib)
- From: Simon McVittie <smcv@debian.org>
- Date: Thu, 8 Mar 2012 14:12:53 +0000
- Message-id: <20120308141253.GA1232@reptile.pseudorandom.co.uk>
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: pu
Tremulous 1.1.0-7 (contrib) is believed to fix CVE-2006-2082, CVE-2006-2236,
CVE-2006-2875, CVE-2006-3324, CVE-2006-3325, CVE-2011-3012, CVE-2011-2764.
The Security Team have indicated that they do not issue DSAs for contrib
packages.
I propose to use a package functionally identical to 1.1.0-7 (differing
only in its changelog and target distribution) as the stable update;
I've avoided making any changes not targeted as a security update.
1.1.0-7~squeeze1 seems like a good version number to represent that,
or I could make it 1.1.0-5+squeeze1 if you prefer.
Changelog since squeeze:
tremulous (1.1.0-7~squeeze1) stable; urgency=low
* Stable update, incorporating security fixes from unstable
* Fix an incorrect bug number in revision -6
-- Simon McVittie <smcv@debian.org> Thu, 08 Mar 2012 13:59:24 +0000
tremulous (1.1.0-7) unstable; urgency=medium
* Add a lintian override for embedded-library libjpeg (#589407) to avoid
auto-rejection. It is a valid bug, but is not a regression, and fixing
several long-standing security vulnerabilities seems more important
than getting rid of an embedded library that is not known to be
exploitable.
-- Simon McVittie <smcv@debian.org> Wed, 22 Feb 2012 10:00:04 +0000
tremulous (1.1.0-6) unstable; urgency=medium
* Backport patches from ioquake3 to fix long-standing security bugs:
- CVE-2006-2082: arbitrary file download from server by a malicious client
(Closes: #660831)
- CVE-2006-2236 ("the remapShader exploit"): missing bounds-checking on
COM_StripExtension, exploitable in clients of a malicious server
(Closes: #660827)
- CVE-2006-2875 ("q3cbof"): buffer overflow in CL_ParseDownload by a
malicious server (Closes: #660830)
- CVE-2006-3324: arbitrary file overwriting in clients of a malicious
server (Closes: #660832)
- CVE-2006-3325: arbitrary cvar overwriting (could lead to arbitrary
code execution) in clients of a malicious server (Closes: #660834)
- CVE-2011-3012, CVE-2011-2764: DLL overwriting (leading to arbitrary
code execution) in clients of a malicious server if auto-downloading
is enabled (Closes: #660836)
* As a precaution, disable auto-downloading
* Backport ioquake3 r1141 to fix a potential buffer overflow in error
handling (not known to be exploitable, but it can't hurt)
* Add gcc attributes to all printf- and scanf-like functions, and
fix non-literal format strings (again, none are known to be exploitable)
-- Simon McVittie <smcv@debian.org> Wed, 22 Feb 2012 09:07:37 +0000
Please find attached:
* filtered.diff: proposed debdiff with the actual patches filtered out,
so you don't have to read diff-of-diffs
(output of: git diff --staged debian/1.1.0-5.. |
filterdiff -p1 --exclude=debian/patches/\*.patch)
* *.patch: the new patches
Regards,
Simon
diff --git a/debian/NEWS b/debian/NEWS
new file mode 100644
index 0000000..50eaafc
--- /dev/null
+++ b/debian/NEWS
@@ -0,0 +1,16 @@
+tremulous (1.1.0-6) unstable; urgency=medium
+
+ This version of Tremulous is based on an older version of the Quake III
+ Arena engine, which has no protection against malicious bytecode programs.
+ It is not safe or secure when used with untrusted PK3 files.
+
+ As a result, the Debian version of Tremulous no longer allows automatic
+ downloading of PK3 files from game servers. The "Auto Download" menu
+ option (the cl_allowDownload variable) still exists, but has no effect,
+ and the engine always behaves as if it was set to 0. You may find that
+ you are unable to join servers that use a modified version of Tremulous.
+
+ Please treat all PK3 files as if they were executable programs, and only
+ install them from trusted sources.
+
+ -- Simon McVittie <smcv@debian.org> Sun, 11 Dec 2011 17:52:26 +0000
diff --git a/debian/changelog b/debian/changelog
index e2f2c24..e110a79 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,45 @@
+tremulous (1.1.0-7~squeeze1) stable; urgency=low
+
+ * Stable update, incorporating security fixes from unstable
+ * Fix an incorrect bug number in revision -6
+
+ -- Simon McVittie <smcv@debian.org> Thu, 08 Mar 2012 13:59:24 +0000
+
+tremulous (1.1.0-7) unstable; urgency=medium
+
+ * Add a lintian override for embedded-library libjpeg (#589407) to avoid
+ auto-rejection. It is a valid bug, but is not a regression, and fixing
+ several long-standing security vulnerabilities seems more important
+ than getting rid of an embedded library that is not known to be
+ exploitable.
+
+ -- Simon McVittie <smcv@debian.org> Wed, 22 Feb 2012 10:00:04 +0000
+
+tremulous (1.1.0-6) unstable; urgency=medium
+
+ * Backport patches from ioquake3 to fix long-standing security bugs:
+ - CVE-2006-2082: arbitrary file download from server by a malicious client
+ (Closes: #660831)
+ - CVE-2006-2236 ("the remapShader exploit"): missing bounds-checking on
+ COM_StripExtension, exploitable in clients of a malicious server
+ (Closes: #660827)
+ - CVE-2006-2875 ("q3cbof"): buffer overflow in CL_ParseDownload by a
+ malicious server (Closes: #660830)
+ - CVE-2006-3324: arbitrary file overwriting in clients of a malicious
+ server (Closes: #660832)
+ - CVE-2006-3325: arbitrary cvar overwriting (could lead to arbitrary
+ code execution) in clients of a malicious server (Closes: #660834)
+ - CVE-2011-3012, CVE-2011-2764: DLL overwriting (leading to arbitrary
+ code execution) in clients of a malicious server if auto-downloading
+ is enabled (Closes: #660836)
+ * As a precaution, disable auto-downloading
+ * Backport ioquake3 r1141 to fix a potential buffer overflow in error
+ handling (not known to be exploitable, but it can't hurt)
+ * Add gcc attributes to all printf- and scanf-like functions, and
+ fix non-literal format strings (again, none are known to be exploitable)
+
+ -- Simon McVittie <smcv@debian.org> Wed, 22 Feb 2012 09:07:37 +0000
+
tremulous (1.1.0-5) unstable; urgency=low
* New maintainer - Debian Games Team
diff --git a/debian/patches/0010-CVE-2006-2082-do-not-allow-download-of-arbitrary-fil.patch b/debian/patches/0010-CVE-2006-2082-do-not-allow-download-of-arbitrary-fil.patch
new file mode 100644
index 0000000..8c42baa
diff --git a/debian/patches/0011-CVE-2006-2236-add-bounds-checking-to-COM_StripExtens.patch b/debian/patches/0011-CVE-2006-2236-add-bounds-checking-to-COM_StripExtens.patch
new file mode 100644
index 0000000..b59c83f
diff --git a/debian/patches/0012-CVE-2006-2875-fix-stack-buffer-overflow-in-CL_ParseD.patch b/debian/patches/0012-CVE-2006-2875-fix-stack-buffer-overflow-in-CL_ParseD.patch
new file mode 100644
index 0000000..319978b
diff --git a/debian/patches/0013-CVE-2006-3324-fix-arbitrary-file-overwrite-on-client.patch b/debian/patches/0013-CVE-2006-3324-fix-arbitrary-file-overwrite-on-client.patch
new file mode 100644
index 0000000..75d51fc
diff --git a/debian/patches/0014-CVE-2006-3325-fix-arbitrary-cvar-overwriting.patch b/debian/patches/0014-CVE-2006-3325-fix-arbitrary-cvar-overwriting.patch
new file mode 100644
index 0000000..e560186
diff --git a/debian/patches/0015-CVE-2011-3012-CVE-2011-2764-backport-from-ioquake3-t.patch b/debian/patches/0015-CVE-2011-3012-CVE-2011-2764-backport-from-ioquake3-t.patch
new file mode 100644
index 0000000..fc64264
diff --git a/debian/patches/0016-Always-behave-as-if-cl_allowDownload-was-false.patch b/debian/patches/0016-Always-behave-as-if-cl_allowDownload-was-false.patch
new file mode 100644
index 0000000..f6967f8
diff --git a/debian/patches/0017-Sys_Error-do-not-overflow-if-an-error-message-exceed.patch b/debian/patches/0017-Sys_Error-do-not-overflow-if-an-error-message-exceed.patch
new file mode 100644
index 0000000..14fb349
diff --git a/debian/patches/0018-Avoid-non-literal-format-strings.patch b/debian/patches/0018-Avoid-non-literal-format-strings.patch
new file mode 100644
index 0000000..05e0408
diff --git a/debian/patches/0019-Annotate-printf-and-scanf-like-functions-with-gcc-at.patch b/debian/patches/0019-Annotate-printf-and-scanf-like-functions-with-gcc-at.patch
new file mode 100644
index 0000000..e54cfbb
diff --git a/debian/patches/series b/debian/patches/series
index 14204cd..f704e87 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,13 @@
0007-Fix-to-disappearing-cursor-on-map-load-Com_Error-bug.patch
0008-Fixed-sort-by-ping.patch
0009-Disable-JIT-QVM-interpreter-on-x86-64.patch
+0010-CVE-2006-2082-do-not-allow-download-of-arbitrary-fil.patch
+0011-CVE-2006-2236-add-bounds-checking-to-COM_StripExtens.patch
+0012-CVE-2006-2875-fix-stack-buffer-overflow-in-CL_ParseD.patch
+0013-CVE-2006-3324-fix-arbitrary-file-overwrite-on-client.patch
+0014-CVE-2006-3325-fix-arbitrary-cvar-overwriting.patch
+0015-CVE-2011-3012-CVE-2011-2764-backport-from-ioquake3-t.patch
+0016-Always-behave-as-if-cl_allowDownload-was-false.patch
+0017-Sys_Error-do-not-overflow-if-an-error-message-exceed.patch
+0018-Avoid-non-literal-format-strings.patch
+0019-Annotate-printf-and-scanf-like-functions-with-gcc-at.patch
diff --git a/debian/tremulous.lintian-overrides b/debian/tremulous.lintian-overrides
new file mode 100644
index 0000000..19135ab
--- /dev/null
+++ b/debian/tremulous.lintian-overrides
@@ -0,0 +1,4 @@
+# This is a valid bug, but has been overridden to get unrelated security
+# fixes into unstable.
+# http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=589407
+tremulous: embedded-library usr/lib/tremulous/tremulous: libjpeg
From: Simon McVittie <smcv@debian.org>
Date: Sat, 18 Feb 2012 20:33:51 +0000
Subject: CVE-2006-2082 - do not allow download of arbitrary files from a
server
Any file readable by the server user could be read, via ../ sequences.
Original patches by Thilo Schulz, ioquake3 r777 (which fixed the
vulnerability) and r781 (which fixed a regression in r777 where
uninitialized variables led to some allowed downloads being rejected too).
Origin: backport
Bug-Debian: http://bugs.debian.org/660831
Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2006-2082
---
src/server/sv_client.c | 51 ++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/src/server/sv_client.c b/src/server/sv_client.c
index 0272fab..53f5998 100644
--- a/src/server/sv_client.c
+++ b/src/server/sv_client.c
@@ -618,24 +618,57 @@ void SV_WriteDownloadToClient( client_t *cl , msg_t *msg )
int curindex;
int rate;
int blockspersnap;
- int idPack, missionPack;
+ int idPack = 0, missionPack = 0, unreferenced = 1;
char errorMessage[1024];
+ char pakbuf[MAX_QPATH], *pakptr;
+ int numRefPaks;
if (!*cl->downloadName)
return; // Nothing being downloaded
if (!cl->download) {
- // We open the file here
+ // Chop off filename extension.
+ Com_sprintf(pakbuf, sizeof(pakbuf), "%s", cl->downloadName);
+ pakptr = Q_strrchr(pakbuf, '.');
- Com_Printf( "clientDownload: %d : begining \"%s\"\n", cl - svs.clients, cl->downloadName );
+ if (pakptr)
+ {
+ *pakptr = '\0';
- missionPack = FS_idPak(cl->downloadName, "missionpack");
- idPack = missionPack || FS_idPak(cl->downloadName, "baseq3");
+ // Check for pk3 filename extension
+ if(!Q_stricmp(pakptr + 1, "pk3"))
+ {
+ const char *referencedPaks = FS_ReferencedPakNames();
+
+ // Check whether the file appears in the list of referenced
+ // paks to prevent downloading of arbitrary files
+ Cmd_TokenizeStringIgnoreQuotes(referencedPaks);
+ numRefPaks = Cmd_Argc();
+
+ for(curindex = 0; curindex < numRefPaks; curindex++)
+ {
+ if(!FS_FilenameCompare(Cmd_Argv(curindex), pakbuf))
+ {
+ unreferenced = 0;
+ // now that we know the file is referenced, check whether it's legal to download it.
+ missionPack = FS_idPak(pakbuf, "missionpack");
+ idPack = missionPack || FS_idPak(pakbuf, BASEGAME);
+ break;
+ }
+ }
+ }
+ }
- if ( !sv_allowDownload->integer || idPack ||
+ // We open the file here
+ if ( !sv_allowDownload->integer || idPack || unreferenced ||
( cl->downloadSize = FS_SV_FOpenFileRead( cl->downloadName, &cl->download ) ) <= 0 ) {
// cannot auto-download file
- if (idPack) {
+ if(unreferenced)
+ {
+ Com_Printf("clientDownload: %d : \"%s\" is not referenced and cannot be downloaded.\n", cl - svs.clients, cl->downloadName);
+ Com_sprintf(errorMessage, sizeof(errorMessage), "File \"%s\" is not referenced and cannot be downloaded.", cl->downloadName);
+ }
+ else if (idPack) {
Com_Printf("clientDownload: %d : \"%s\" cannot download id pk3 files\n", cl - svs.clients, cl->downloadName);
if (missionPack) {
Com_sprintf(errorMessage, sizeof(errorMessage), "Cannot autodownload Team Arena file \"%s\"\n"
@@ -670,7 +703,9 @@ void SV_WriteDownloadToClient( client_t *cl , msg_t *msg )
*cl->downloadName = 0;
return;
}
-
+
+ Com_Printf( "clientDownload: %d : beginning \"%s\"\n", cl - svs.clients, cl->downloadName );
+
// Init
cl->downloadCurrentBlock = cl->downloadClientBlock = cl->downloadXmitBlock = 0;
cl->downloadCount = 0;
From: Simon McVittie <smcv@debian.org>
Date: Fri, 18 Nov 2011 18:44:44 +0000
Subject: CVE-2006-2236 - add bounds-checking to COM_StripExtension
This fixes the "remapShader" exploit by backporting ioquake3 r765, with
a further change to avoid strncpy'ing a string into itself.
Original patch by Thilo Schulz.
Origin: backport
Bug-Debian: http://bugs.debian.org/660827
Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2006-2236
Bug-Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=455458
---
src/cgame/cg_weapons.c | 6 +++---
src/client/cl_main.c | 2 +-
src/qcommon/files.c | 2 +-
src/qcommon/q_shared.c | 6 ++++--
src/qcommon/q_shared.h | 2 +-
src/qcommon/vm.c | 2 +-
src/renderer/tr_bsp.c | 2 +-
src/renderer/tr_shader.c | 6 +++---
src/ui/ui_main.c | 2 +-
src/ui/ui_players.c | 4 ++--
10 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/src/cgame/cg_weapons.c b/src/cgame/cg_weapons.c
index ecb2499..87a9b4c 100644
--- a/src/cgame/cg_weapons.c
+++ b/src/cgame/cg_weapons.c
@@ -503,17 +503,17 @@ static qboolean CG_ParseWeaponFile( const char *filename, weaponInfo_t *wi )
CG_Printf( S_COLOR_RED "ERROR: weapon model not found %s\n", token );
strcpy( path, token );
- COM_StripExtension( path, path );
+ COM_StripExtension( path, path, sizeof(path) );
strcat( path, "_flash.md3" );
wi->flashModel = trap_R_RegisterModel( path );
strcpy( path, token );
- COM_StripExtension( path, path );
+ COM_StripExtension( path, path, sizeof(path) );
strcat( path, "_barrel.md3" );
wi->barrelModel = trap_R_RegisterModel( path );
strcpy( path, token );
- COM_StripExtension( path, path );
+ COM_StripExtension( path, path, sizeof(path) );
strcat( path, "_hand.md3" );
wi->handsModel = trap_R_RegisterModel( path );
diff --git a/src/client/cl_main.c b/src/client/cl_main.c
index 3db1d6d..7b562bd 100644
--- a/src/client/cl_main.c
+++ b/src/client/cl_main.c
@@ -2014,7 +2014,7 @@ void CL_Frame ( int msec ) {
}
Q_strncpyz( mapName, COM_SkipPath( cl.mapname ), sizeof( cl.mapname ) );
- COM_StripExtension( mapName, mapName );
+ COM_StripExtension(mapName, mapName, sizeof(mapName));
Cbuf_ExecuteText( EXEC_NOW,
va( "record %s-%s-%s", nowString, serverName, mapName ) );
diff --git a/src/qcommon/files.c b/src/qcommon/files.c
index 84db5b1..67f375c 100644
--- a/src/qcommon/files.c
+++ b/src/qcommon/files.c
@@ -3377,7 +3377,7 @@ void FS_FilenameCompletion( const char *dir, const char *ext,
Q_strncpyz( filename, filenames[ i ], MAX_STRING_CHARS );
if( stripExt ) {
- COM_StripExtension( filename, filename );
+ COM_StripExtension(filename, filename, sizeof(filename));
}
callback( filename );
diff --git a/src/qcommon/q_shared.c b/src/qcommon/q_shared.c
index 9f152d6..44085cc 100644
--- a/src/qcommon/q_shared.c
+++ b/src/qcommon/q_shared.c
@@ -59,10 +59,12 @@ char *COM_SkipPath (char *pathname)
COM_StripExtension
============
*/
-void COM_StripExtension( const char *in, char *out ) {
+void COM_StripExtension( const char *in, char *out, int destsize ) {
int length;
- strcpy( out, in );
+ if (in != out) {
+ Q_strncpyz(out, in, destsize);
+ }
length = strlen(out)-1;
while (length > 0 && out[length] != '.')
diff --git a/src/qcommon/q_shared.h b/src/qcommon/q_shared.h
index 0059cdf..03b2e03 100644
--- a/src/qcommon/q_shared.h
+++ b/src/qcommon/q_shared.h
@@ -627,7 +627,7 @@ vec_t DistanceBetweenLineSegments(
float Com_Clamp( float min, float max, float value );
char *COM_SkipPath( char *pathname );
-void COM_StripExtension( const char *in, char *out );
+void COM_StripExtension(const char *in, char *out, int destsize);
void COM_DefaultExtension( char *path, int maxSize, const char *extension );
void COM_BeginParseSession( const char *name );
diff --git a/src/qcommon/vm.c b/src/qcommon/vm.c
index 0235ffe..8fb588d 100644
--- a/src/qcommon/vm.c
+++ b/src/qcommon/vm.c
@@ -231,7 +231,7 @@ void VM_LoadSymbols( vm_t *vm ) {
return;
}
- COM_StripExtension( vm->name, name );
+ COM_StripExtension(vm->name, name, sizeof(name));
Com_sprintf( symbols, sizeof( symbols ), "vm/%s.map", name );
len = FS_ReadFile( symbols, (void **)&mapfile );
if ( !mapfile ) {
diff --git a/src/renderer/tr_bsp.c b/src/renderer/tr_bsp.c
index eb57da8..2eff834 100644
--- a/src/renderer/tr_bsp.c
+++ b/src/renderer/tr_bsp.c
@@ -1824,7 +1824,7 @@ void RE_LoadWorldMap( const char *name ) {
Q_strncpyz( s_worldData.name, name, sizeof( s_worldData.name ) );
Q_strncpyz( s_worldData.baseName, COM_SkipPath( s_worldData.name ), sizeof( s_worldData.name ) );
- COM_StripExtension( s_worldData.baseName, s_worldData.baseName );
+ COM_StripExtension(s_worldData.baseName, s_worldData.baseName, sizeof(s_worldData.baseName));
startMarker = ri.Hunk_Alloc(0, h_low);
c_gridVerts = 0;
diff --git a/src/renderer/tr_shader.c b/src/renderer/tr_shader.c
index 380f9bf..c349b15 100644
--- a/src/renderer/tr_shader.c
+++ b/src/renderer/tr_shader.c
@@ -96,7 +96,7 @@ void R_RemapShader(const char *shaderName, const char *newShaderName, const char
// remap all the shaders with the given name
// even tho they might have different lightmaps
- COM_StripExtension( shaderName, strippedName );
+ COM_StripExtension(shaderName, strippedName, sizeof(strippedName));
hash = generateHashValue(strippedName, FILE_HASH_SIZE);
for (sh = hashTable[hash]; sh; sh = sh->next) {
if (Q_stricmp(sh->name, strippedName) == 0) {
@@ -2366,7 +2366,7 @@ shader_t *R_FindShaderByName( const char *name ) {
return tr.defaultShader;
}
- COM_StripExtension( name, strippedName );
+ COM_StripExtension(name, strippedName, sizeof(strippedName));
hash = generateHashValue(strippedName, FILE_HASH_SIZE);
@@ -2434,7 +2434,7 @@ shader_t *R_FindShader( const char *name, int lightmapIndex, qboolean mipRawImag
lightmapIndex = LIGHTMAP_BY_VERTEX;
}
- COM_StripExtension( name, strippedName );
+ COM_StripExtension(name, strippedName, sizeof(strippedName));
hash = generateHashValue(strippedName, FILE_HASH_SIZE);
diff --git a/src/ui/ui_main.c b/src/ui/ui_main.c
index 7509d9b..604e709 100644
--- a/src/ui/ui_main.c
+++ b/src/ui/ui_main.c
@@ -5376,7 +5376,7 @@ static void UI_BuildQ3Model_List( void )
{
filelen = strlen(fileptr);
- COM_StripExtension(fileptr,skinname);
+ COM_StripExtension(fileptr, skinname, sizeof(skinname));
// look for icon_????
if (Q_stricmpn(skinname, "icon_", 5) == 0 && !(Q_stricmp(skinname,"icon_blue") == 0 || Q_stricmp(skinname,"icon_red") == 0))
diff --git a/src/ui/ui_players.c b/src/ui/ui_players.c
index 5dbfdd3..f7baf31 100644
--- a/src/ui/ui_players.c
+++ b/src/ui/ui_players.c
@@ -84,13 +84,13 @@ tryagain:
if ( weaponNum == WP_MACHINEGUN ) {
strcpy( path, item->world_model[0] );
- COM_StripExtension( path, path );
+ COM_StripExtension( path, path, sizeof(path) );
strcat( path, "_barrel.md3" );
pi->barrelModel = trap_R_RegisterModel( path );
}
strcpy( path, item->world_model[0] );
- COM_StripExtension( path, path );
+ COM_StripExtension( path, path, sizeof(path) );
strcat( path, "_flash.md3" );
pi->flashModel = trap_R_RegisterModel( path );
From: Simon McVittie <smcv@debian.org>
Date: Fri, 18 Nov 2011 20:56:32 +0000
Subject: CVE-2006-2875 - fix stack buffer overflow in CL_ParseDownload
This is exploitable by a modified server. Original patch by Thilo
Schulz, ioquake3 r796.
Origin: backport
Bug-Debian: http://bugs.debian.org/660830
Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2006-2875
---
src/client/cl_parse.c | 28 ++++++++++++++++++++--------
1 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/client/cl_parse.c b/src/client/cl_parse.c
index 7f219b3..dc14cd6 100644
--- a/src/client/cl_parse.c
+++ b/src/client/cl_parse.c
@@ -256,6 +256,13 @@ void CL_ParseSnapshot( msg_t *msg ) {
// read areamask
len = MSG_ReadByte( msg );
+
+ if(len > sizeof(newSnap.areamask))
+ {
+ Com_Error (ERR_DROP,"CL_ParseSnapshot: Invalid size %d for areamask.", len);
+ return;
+ }
+
MSG_ReadData( msg, &newSnap.areamask, len);
// read playerinfo
@@ -476,6 +483,12 @@ void CL_ParseDownload ( msg_t *msg ) {
unsigned char data[MAX_MSGLEN];
int block;
+ if (!*clc.downloadTempName) {
+ Com_Printf("Server sending download, but no download was requested\n");
+ CL_AddReliableCommand( "stopdl" );
+ return;
+ }
+
// read the data
block = MSG_ReadShort ( msg );
@@ -494,8 +507,13 @@ void CL_ParseDownload ( msg_t *msg ) {
}
size = MSG_ReadShort ( msg );
- if (size > 0)
- MSG_ReadData( msg, data, size );
+ if (size < 0 || size > sizeof(data))
+ {
+ Com_Error(ERR_DROP, "CL_ParseDownload: Invalid size %d for download chunk.", size);
+ return;
+ }
+
+ MSG_ReadData(msg, data, size);
if (clc.downloadBlock != block) {
Com_DPrintf( "CL_ParseDownload: Expected block %d, got %d\n", clc.downloadBlock, block);
@@ -505,12 +523,6 @@ void CL_ParseDownload ( msg_t *msg ) {
// open the file if not opened yet
if (!clc.download)
{
- if (!*clc.downloadTempName) {
- Com_Printf("Server sending download, but no download was requested\n");
- CL_AddReliableCommand( "stopdl" );
- return;
- }
-
clc.download = FS_SV_FOpenFileWrite( clc.downloadTempName );
if (!clc.download) {
From: Simon McVittie <smcv@debian.org>
Date: Mon, 20 Feb 2012 21:57:46 +0000
Subject: CVE-2006-3324 - fix arbitrary file overwrite on client by malicious
server
Original patches by Thilo Schulz, ioquake3 r790, r794, r804.
This commit also includes "a few sanity checks for checksum/pakname storage
to fix a crash that can occur under certain circumstances", from r804
and r805.
Bug-Debian: http://bugs.debian.org/660832
Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2006-3324
Origin: backport
---
src/qcommon/files.c | 51 +++++++++++++++++++++++++++++++++++++++------------
1 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/src/qcommon/files.c b/src/qcommon/files.c
index 67f375c..34afb4b 100644
--- a/src/qcommon/files.c
+++ b/src/qcommon/files.c
@@ -2591,15 +2591,16 @@ we are not interested in a download string format, we want something human-reada
qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
searchpath_t *sp;
qboolean havepak, badchecksum;
+ char *origpos = neededpaks;
int i;
- if ( !fs_numServerReferencedPaks ) {
+ if (!fs_numServerReferencedPaks)
return qfalse; // Server didn't send any pack information along
- }
*neededpaks = 0;
- for ( i = 0 ; i < fs_numServerReferencedPaks ; i++ ) {
+ for ( i = 0 ; i < fs_numServerReferencedPaks ; i++ )
+ {
// Ok, see if we have this pak file
badchecksum = qfalse;
havepak = qfalse;
@@ -2609,6 +2610,13 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
continue;
}
+ // Make sure the server cannot make us write to non-quake3 directories.
+ if(strstr(fs_serverReferencedPakNames[i], "../") || strstr(fs_serverReferencedPakNames[i], "..\\"))
+ {
+ Com_Printf("WARNING: Invalid download name %s\n", fs_serverReferencedPakNames[i]);
+ continue;
+ }
+
for ( sp = fs_searchpaths ; sp ; sp = sp->next ) {
if ( sp->pack && sp->pack->checksum == fs_serverReferencedPaks[i] ) {
havepak = qtrue; // This is it!
@@ -2621,6 +2629,12 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
if (dlstring)
{
+ // We need this to make sure we won't hit the end of the buffer or the server could
+ // overwrite non-pk3 files on clients by writing so much crap into neededpaks that
+ // Q_strcat cuts off the .pk3 extension.
+
+ origpos += strlen(origpos);
+
// Remote name
Q_strcat( neededpaks, len, "@");
Q_strcat( neededpaks, len, fs_serverReferencedPakNames[i] );
@@ -2641,6 +2655,14 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
Q_strcat( neededpaks, len, fs_serverReferencedPakNames[i] );
Q_strcat( neededpaks, len, ".pk3" );
}
+
+ // Find out whether it might have overflowed the buffer and don't add this file to the
+ // list if that is the case.
+ if(strlen(origpos) + (origpos - neededpaks) >= len - 1)
+ {
+ *origpos = '\0';
+ break;
+ }
}
else
{
@@ -3146,7 +3168,7 @@ checksums to see if any pk3 files need to be auto-downloaded.
=====================
*/
void FS_PureServerSetReferencedPaks( const char *pakSums, const char *pakNames ) {
- int i, c, d;
+ int i, c, d = 0;
Cmd_TokenizeString( pakSums );
@@ -3155,30 +3177,35 @@ void FS_PureServerSetReferencedPaks( const char *pakSums, const char *pakNames )
c = MAX_SEARCH_PATHS;
}
- fs_numServerReferencedPaks = c;
-
for ( i = 0 ; i < c ; i++ ) {
fs_serverReferencedPaks[i] = atoi( Cmd_Argv( i ) );
}
- for ( i = 0 ; i < c ; i++ ) {
- if (fs_serverReferencedPakNames[i]) {
+ for (i = 0 ; i < sizeof(fs_serverReferencedPakNames) / sizeof(*fs_serverReferencedPakNames); i++)
+ {
+ if(fs_serverReferencedPakNames[i])
Z_Free(fs_serverReferencedPakNames[i]);
- }
+
fs_serverReferencedPakNames[i] = NULL;
}
+
if ( pakNames && *pakNames ) {
Cmd_TokenizeString( pakNames );
d = Cmd_Argc();
- if ( d > MAX_SEARCH_PATHS ) {
- d = MAX_SEARCH_PATHS;
- }
+ if(d > c)
+ d = c;
for ( i = 0 ; i < d ; i++ ) {
fs_serverReferencedPakNames[i] = CopyString( Cmd_Argv( i ) );
}
}
+
+ // ensure that there are as many checksums as there are pak names.
+ if(d < c)
+ c = d;
+
+ fs_numServerReferencedPaks = c;
}
/*
From: Simon McVittie <smcv@debian.org>
Date: Fri, 18 Nov 2011 21:03:07 +0000
Subject: CVE-2006-3325: fix arbitrary cvar overwriting
Original patch by Thilo Schulz, ioquake3 r811.
Bug-Debian: http://bugs.debian.org/660834
Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2006-3325
Origin: backport
---
src/client/cl_parse.c | 23 +++++++++++++++++++++--
src/qcommon/cvar.c | 14 ++++++++++++++
src/qcommon/files.c | 19 ++++++++++++++++++-
src/qcommon/q_shared.h | 3 +++
src/qcommon/qcommon.h | 4 ++++
5 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/src/client/cl_parse.c b/src/client/cl_parse.c
index dc14cd6..2d36aa1 100644
--- a/src/client/cl_parse.c
+++ b/src/client/cl_parse.c
@@ -369,16 +369,35 @@ void CL_SystemInfoChanged( void ) {
// scan through all the variables in the systeminfo and locally set cvars to match
s = systemInfo;
while ( s ) {
+ int cvar_flags;
+
Info_NextPair( &s, key, value );
if ( !key[0] ) {
break;
}
+
// ehw!
- if ( !Q_stricmp( key, "fs_game" ) ) {
+ if (!Q_stricmp(key, "fs_game"))
+ {
+ if(FS_CheckDirTraversal(value))
+ {
+ Com_Printf("WARNING: Server sent invalid fs_game value %s\n", value);
+ continue;
+ }
+
gameSet = qtrue;
}
- Cvar_Set( key, value );
+ if((cvar_flags = Cvar_Flags(key)) == CVAR_NONEXISTENT)
+ Cvar_Get(key, value, CVAR_SERVER_CREATED | CVAR_ROM);
+ else
+ {
+ // If this cvar may not be modified by a server discard the value.
+ if(!(cvar_flags & (CVAR_SYSTEMINFO | CVAR_SERVER_CREATED)))
+ continue;
+
+ Cvar_Set(key, value);
+ }
}
// if game folder should not be set and it is set at the client side
if ( !gameSet && *Cvar_VariableString("fs_game") ) {
diff --git a/src/qcommon/cvar.c b/src/qcommon/cvar.c
index a306d88..f6caea8 100644
--- a/src/qcommon/cvar.c
+++ b/src/qcommon/cvar.c
@@ -162,6 +162,20 @@ void Cvar_VariableStringBuffer( const char *var_name, char *buffer, int bufsize
}
}
+/*
+============
+Cvar_Flags
+============
+*/
+int Cvar_Flags(const char *var_name)
+{
+ cvar_t *var;
+
+ if(! (var = Cvar_FindVar(var_name)) )
+ return CVAR_NONEXISTENT;
+ else
+ return var->flags;
+}
/*
============
diff --git a/src/qcommon/files.c b/src/qcommon/files.c
index 34afb4b..49390a0 100644
--- a/src/qcommon/files.c
+++ b/src/qcommon/files.c
@@ -2564,6 +2564,23 @@ qboolean FS_idPak( char *pak, char *base ) {
/*
================
+FS_idPak
+
+Check whether the string contains stuff like "../" to prevent directory traversal bugs
+and return qtrue if it does.
+================
+*/
+
+qboolean FS_CheckDirTraversal(const char *checkdir)
+{
+ if(strstr(checkdir, "../") || strstr(checkdir, "..\\"))
+ return qtrue;
+
+ return qfalse;
+}
+
+/*
+================
FS_ComparePaks
----------------
@@ -2611,7 +2628,7 @@ qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring ) {
}
// Make sure the server cannot make us write to non-quake3 directories.
- if(strstr(fs_serverReferencedPakNames[i], "../") || strstr(fs_serverReferencedPakNames[i], "..\\"))
+ if(FS_CheckDirTraversal(fs_serverReferencedPakNames[i]))
{
Com_Printf("WARNING: Invalid download name %s\n", fs_serverReferencedPakNames[i]);
continue;
diff --git a/src/qcommon/q_shared.h b/src/qcommon/q_shared.h
index 03b2e03..d491708 100644
--- a/src/qcommon/q_shared.h
+++ b/src/qcommon/q_shared.h
@@ -795,6 +795,9 @@ default values.
#define CVAR_CHEAT 512 // can not be changed if cheats are disabled
#define CVAR_NORESTART 1024 // do not clear when a cvar_restart is issued
+#define CVAR_SERVER_CREATED 2048 // cvar was created by a server the client connected to.
+#define CVAR_NONEXISTENT 0xFFFFFFFF // Cvar doesn't exist.
+
// nothing outside the Cvar_*() functions should modify these fields!
typedef struct cvar_s {
char *name;
diff --git a/src/qcommon/qcommon.h b/src/qcommon/qcommon.h
index e6ae058..7b2fb8a 100644
--- a/src/qcommon/qcommon.h
+++ b/src/qcommon/qcommon.h
@@ -477,6 +477,9 @@ char *Cvar_VariableString( const char *var_name );
void Cvar_VariableStringBuffer( const char *var_name, char *buffer, int bufsize );
// returns an empty string if not defined
+int Cvar_Flags(const char *var_name);
+// returns CVAR_NONEXISTENT if cvar doesn't exist or the flags of that particular CVAR.
+
void Cvar_CommandCompletion( void(*callback)(const char *s) );
// callback with each valid string
@@ -647,6 +650,7 @@ void FS_PureServerSetLoadedPaks( const char *pakSums, const char *pakNames );
// separated checksums will be checked for files, with the
// sole exception of .cfg files.
+qboolean FS_CheckDirTraversal(const char *checkdir);
qboolean FS_idPak( char *pak, char *base );
qboolean FS_ComparePaks( char *neededpaks, int len, qboolean dlstring );
From: Simon McVittie <smcv@debian.org>
Date: Thu, 16 Feb 2012 20:28:58 +0000
Subject: CVE-2011-3012, CVE-2011-2764 - backport from ioquake3 to prevent DLL
overwriting
This is a backport of several patches:
* part of ioquake3 r1405, from TsT (attempt to prevent DLL overwriting,
CVE-2011-3012)
* part of ioquake3 r1456, from Patrick Baggett (using __func__)
* ioquake3 r1499, from Tim Angus (fix potential buffer underrun)
* ioquake3 r2098, from Thilo Schulz (fix incomplete DLL overwrite prevention
in previous commits, CVE-2011-2764)
Origin: backport
Bug-Debian: http://bugs.debian.org/660836
Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2011-3012
Bug-CVE: http://security-tracker.debian.org/tracker/CVE-2011-2764
---
src/qcommon/files.c | 34 ++++++++++++++++++++++++++++++++++
src/qcommon/q_shared.c | 24 ++++++++++++++++++++++++
src/qcommon/q_shared.h | 1 +
3 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/src/qcommon/files.c b/src/qcommon/files.c
index 49390a0..2e28cab 100644
--- a/src/qcommon/files.c
+++ b/src/qcommon/files.c
@@ -510,6 +510,24 @@ static qboolean FS_CreatePath (char *OSPath) {
/*
=================
+FS_CheckFilenameIsNotExecutable
+
+ERR_FATAL if trying to maniuplate a file with the platform library extension
+=================
+ */
+static void FS_CheckFilenameIsNotExecutable( const char *filename, const char *function )
+{
+ // Check if the filename ends with the library extension
+ if(COM_CompareExtension(filename, DLL_EXT))
+ {
+ Com_Error( ERR_FATAL, "%s: Not allowed to manipulate '%s' due "
+ "to %s extension\n", function, filename, DLL_EXT );
+ }
+}
+
+
+/*
+=================
FS_CopyFile
Copy a fully specified file from one place to another
@@ -522,6 +540,8 @@ static void FS_CopyFile( char *fromOSPath, char *toOSPath ) {
Com_Printf( "copy %s to %s\n", fromOSPath, toOSPath );
+ FS_CheckFilenameIsNotExecutable( toOSPath, __func__ );
+
if (strstr(fromOSPath, "journal.dat") || strstr(fromOSPath, "journaldata.dat")) {
Com_Printf( "Ignoring journal files\n");
return;
@@ -563,6 +583,8 @@ FS_Remove
===========
*/
void FS_Remove( const char *osPath ) {
+ FS_CheckFilenameIsNotExecutable( osPath, __func__ );
+
remove( osPath );
}
@@ -573,6 +595,8 @@ FS_HomeRemove
===========
*/
void FS_HomeRemove( const char *homePath ) {
+ FS_CheckFilenameIsNotExecutable( homePath, __func__ );
+
remove( FS_BuildOSPath( fs_homepath->string,
fs_gamedir, homePath ) );
}
@@ -650,6 +674,8 @@ fileHandle_t FS_SV_FOpenFileWrite( const char *filename ) {
Com_Printf( "FS_SV_FOpenFileWrite: %s\n", ospath );
}
+ FS_CheckFilenameIsNotExecutable( ospath, __func__ );
+
if( FS_CreatePath( ospath ) ) {
return 0;
}
@@ -775,6 +801,8 @@ void FS_SV_Rename( const char *from, const char *to ) {
Com_Printf( "FS_SV_Rename: %s --> %s\n", from_ospath, to_ospath );
}
+ FS_CheckFilenameIsNotExecutable( to_ospath, __func__ );
+
if (rename( from_ospath, to_ospath )) {
// Failed, try copying it and deleting the original
FS_CopyFile ( from_ospath, to_ospath );
@@ -807,6 +835,8 @@ void FS_Rename( const char *from, const char *to ) {
Com_Printf( "FS_Rename: %s --> %s\n", from_ospath, to_ospath );
}
+ FS_CheckFilenameIsNotExecutable( to_ospath, __func__ );
+
if (rename( from_ospath, to_ospath )) {
// Failed, try copying it and deleting the original
FS_CopyFile ( from_ospath, to_ospath );
@@ -871,6 +901,8 @@ fileHandle_t FS_FOpenFileWrite( const char *filename ) {
Com_Printf( "FS_FOpenFileWrite: %s\n", ospath );
}
+ FS_CheckFilenameIsNotExecutable( ospath, __func__ );
+
if( FS_CreatePath( ospath ) ) {
return 0;
}
@@ -917,6 +949,8 @@ fileHandle_t FS_FOpenFileAppend( const char *filename ) {
Com_Printf( "FS_FOpenFileAppend: %s\n", ospath );
}
+ FS_CheckFilenameIsNotExecutable( ospath, __func__ );
+
if( FS_CreatePath( ospath ) ) {
return 0;
}
diff --git a/src/qcommon/q_shared.c b/src/qcommon/q_shared.c
index 44085cc..c87b44b 100644
--- a/src/qcommon/q_shared.c
+++ b/src/qcommon/q_shared.c
@@ -77,6 +77,30 @@ void COM_StripExtension( const char *in, char *out, int destsize ) {
out[length] = 0;
}
+/*
+============
+COM_CompareExtension
+
+string compare the end of the strings and return qtrue if strings match
+============
+*/
+qboolean COM_CompareExtension(const char *in, const char *ext)
+{
+ int inlen, extlen;
+
+ inlen = strlen(in);
+ extlen = strlen(ext);
+
+ if(extlen <= inlen)
+ {
+ in += inlen - extlen;
+
+ if(!Q_stricmp(in, ext))
+ return qtrue;
+ }
+
+ return qfalse;
+}
/*
==================
diff --git a/src/qcommon/q_shared.h b/src/qcommon/q_shared.h
index d491708..83f5789 100644
--- a/src/qcommon/q_shared.h
+++ b/src/qcommon/q_shared.h
@@ -628,6 +628,7 @@ float Com_Clamp( float min, float max, float value );
char *COM_SkipPath( char *pathname );
void COM_StripExtension(const char *in, char *out, int destsize);
+qboolean COM_CompareExtension(const char *in, const char *ext);
void COM_DefaultExtension( char *path, int maxSize, const char *extension );
void COM_BeginParseSession( const char *name );
From: Simon McVittie <smcv@debian.org>
Date: Mon, 20 Feb 2012 23:03:45 +0000
Subject: Always behave as if cl_allowDownload was false
Even in current versions of ioquake3, it is not at all obvious whether
running untrusted bytecode is safe. In this older version, it's certainly
not safe, so let's knock out auto-downloading functionality.
Origin: vendor, Debian
Forwarded: no, Debian-specific removal of functionality
---
src/client/cl_main.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/client/cl_main.c b/src/client/cl_main.c
index 7b562bd..8b96dd4 100644
--- a/src/client/cl_main.c
+++ b/src/client/cl_main.c
@@ -1430,7 +1430,10 @@ and determine if we need to download them
void CL_InitDownloads(void) {
char missingfiles[1024];
- if ( !cl_allowDownload->integer )
+ // Debian-specific patch to allow for longer-term support: auto-downloading
+ // is not safe in this version of the engine, so always behave as if
+ // cl_allowDownload had been disabled.
+ if ( 1 )
{
// autodownload is disabled on the client
// but it's possible that some referenced files on the server are missing
@@ -1440,7 +1443,7 @@ void CL_InitDownloads(void) {
// but at this point while joining the game we don't know wether we will successfully join or not
Com_Printf( "\nWARNING: You are missing some files referenced by the server:\n%s"
"You might not be able to join the game\n"
- "Go to the setting menu to turn on autodownload, or get the file elsewhere\n\n", missingfiles );
+ "\n", missingfiles );
}
}
else if ( FS_ComparePaks( clc.downloadList, sizeof( clc.downloadList ) , qtrue ) ) {
From: Simon McVittie <smcv@debian.org>
Date: Sun, 19 Feb 2012 22:25:33 +0000
Subject: Sys_Error: do not overflow if an error message exceeds 1024
characters
Backport of ioquake3 r1141 by Thilo Schulz. Not known to be exploitable,
but it can't hurt.
If this turns out to be exploitable, please mention ioquake3 r1141
prominently in any advisory.
Origin: backport
---
src/unix/unix_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/unix/unix_main.c b/src/unix/unix_main.c
index eb81568..375d76e 100644
--- a/src/unix/unix_main.c
+++ b/src/unix/unix_main.c
@@ -437,7 +437,7 @@ void Sys_Error( const char *error, ...)
CL_Shutdown ();
va_start (argptr,error);
- vsprintf (string,error,argptr);
+ Q_vsnprintf (string, sizeof(string), error, argptr);
va_end (argptr);
fprintf(stderr, "Sys_Error: %s\n", string);
From: Simon McVittie <smcv@debian.org>
Date: Sun, 19 Feb 2012 23:16:49 +0000
Subject: Avoid non-literal format strings
This is a precautionary measure against potential exploits; none of these
instances is known to be exploitable.
Origin: vendor, Debian
---
src/botlib/be_aas_main.c | 2 +-
src/botlib/l_script.c | 2 +-
src/client/cl_cgame.c | 2 +-
src/client/cl_main.c | 2 +-
src/client/cl_parse.c | 2 +-
src/game/g_combat.c | 6 +++---
src/ui/ui_main.c | 4 ++--
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/botlib/be_aas_main.c b/src/botlib/be_aas_main.c
index 264c784..3a9a569 100644
--- a/src/botlib/be_aas_main.c
+++ b/src/botlib/be_aas_main.c
@@ -63,7 +63,7 @@ void QDECL AAS_Error(char *fmt, ...)
va_start(arglist, fmt);
vsprintf(str, fmt, arglist);
va_end(arglist);
- botimport.Print(PRT_FATAL, str);
+ botimport.Print(PRT_FATAL, "%s", str);
} //end of the function AAS_Error
//===========================================================================
//
diff --git a/src/botlib/l_script.c b/src/botlib/l_script.c
index 7b2e2ad..749afd4 100644
--- a/src/botlib/l_script.c
+++ b/src/botlib/l_script.c
@@ -1429,6 +1429,6 @@ void PS_SetBaseFolder(char *path)
#ifdef BSPC
sprintf(basefolder, path);
#else
- Com_sprintf(basefolder, sizeof(basefolder), path);
+ Com_sprintf(basefolder, sizeof(basefolder), "%s", path);
#endif
} //end of the function PS_SetBaseFolder
diff --git a/src/client/cl_cgame.c b/src/client/cl_cgame.c
index 7d4c0a9..e9c341e 100644
--- a/src/client/cl_cgame.c
+++ b/src/client/cl_cgame.c
@@ -298,7 +298,7 @@ rescan:
// https://zerowing.idsoftware.com/bugzilla/show_bug.cgi?id=552
// allow server to indicate why they were disconnected
if ( argc >= 2 )
- Com_Error (ERR_SERVERDISCONNECT, va( "Server Disconnected - %s", Cmd_Argv( 1 ) ) );
+ Com_Error (ERR_SERVERDISCONNECT, "Server Disconnected - %s", Cmd_Argv( 1 ) );
else
Com_Error (ERR_SERVERDISCONNECT,"Server disconnected\n");
}
diff --git a/src/client/cl_main.c b/src/client/cl_main.c
index 78cf9e7..b4e2c23 100644
--- a/src/client/cl_main.c
+++ b/src/client/cl_main.c
@@ -2967,7 +2967,7 @@ void CL_GlobalServers_f( void ) {
buffptr += sprintf( buffptr, " demo" );
}
- NET_OutOfBandPrint( NS_SERVER, to, command );
+ NET_OutOfBandPrint( NS_SERVER, to, "%s", command );
}
diff --git a/src/client/cl_parse.c b/src/client/cl_parse.c
index 2d36aa1..23f82ea 100644
--- a/src/client/cl_parse.c
+++ b/src/client/cl_parse.c
@@ -520,7 +520,7 @@ void CL_ParseDownload ( msg_t *msg ) {
if (clc.downloadSize < 0)
{
- Com_Error(ERR_DROP, MSG_ReadString( msg ) );
+ Com_Error(ERR_DROP, "%s", MSG_ReadString( msg ) );
return;
}
}
diff --git a/src/game/g_combat.c b/src/game/g_combat.c
index 7e38f11..1f48bba 100644
--- a/src/game/g_combat.c
+++ b/src/game/g_combat.c
@@ -831,13 +831,13 @@ void G_InitDamageLocations( void )
len = trap_FS_FOpenFile( filename, &fileHandle, FS_READ );
if ( !fileHandle )
{
- G_Printf( va( S_COLOR_RED "file not found: %s\n", filename ) );
+ G_Printf( S_COLOR_RED "file not found: %s\n", filename );
continue;
}
if( len >= MAX_LOCDAMAGE_TEXT )
{
- G_Printf( va( S_COLOR_RED "file too large: %s is %i, max allowed is %i", filename, len, MAX_LOCDAMAGE_TEXT ) );
+ G_Printf( S_COLOR_RED "file too large: %s is %i, max allowed is %i", filename, len, MAX_LOCDAMAGE_TEXT );
trap_FS_FCloseFile( fileHandle );
continue;
}
@@ -862,7 +862,7 @@ void G_InitDamageLocations( void )
if( len >= MAX_LOCDAMAGE_TEXT )
{
- G_Printf( va( S_COLOR_RED "file too large: %s is %i, max allowed is %i", filename, len, MAX_LOCDAMAGE_TEXT ) );
+ G_Printf( S_COLOR_RED "file too large: %s is %i, max allowed is %i", filename, len, MAX_LOCDAMAGE_TEXT );
trap_FS_FCloseFile( fileHandle );
continue;
}
diff --git a/src/ui/ui_main.c b/src/ui/ui_main.c
index 604e709..ee60f0f 100644
--- a/src/ui/ui_main.c
+++ b/src/ui/ui_main.c
@@ -5382,7 +5382,7 @@ static void UI_BuildQ3Model_List( void )
if (Q_stricmpn(skinname, "icon_", 5) == 0 && !(Q_stricmp(skinname,"icon_blue") == 0 || Q_stricmp(skinname,"icon_red") == 0))
{
if (Q_stricmp(skinname, "icon_default") == 0) {
- Com_sprintf( scratch, sizeof(scratch), dirptr);
+ Com_sprintf( scratch, sizeof(scratch), "%s", dirptr);
} else {
Com_sprintf( scratch, sizeof(scratch), "%s/%s",dirptr, skinname + 5);
}
@@ -5394,7 +5394,7 @@ static void UI_BuildQ3Model_List( void )
}
}
if (!dirty) {
- Com_sprintf( uiInfo.q3HeadNames[uiInfo.q3HeadCount], sizeof(uiInfo.q3HeadNames[uiInfo.q3HeadCount]), scratch);
+ Com_sprintf( uiInfo.q3HeadNames[uiInfo.q3HeadCount], sizeof(uiInfo.q3HeadNames[uiInfo.q3HeadCount]), "%s", scratch);
uiInfo.q3HeadIcons[uiInfo.q3HeadCount++] = trap_R_RegisterShaderNoMip(va("models/players/%s/%s",dirptr,skinname));
}
}
From: Simon McVittie <smcv@debian.org>
Date: Sun, 19 Feb 2012 23:18:28 +0000
Subject: Annotate printf- and scanf-like functions with gcc attributes
This isn't necessarily suitable for upstream (non-portable) but it
makes -Werror=format-security work better.
Origin: vendor, Debian
---
src/botlib/be_aas_main.h | 2 +-
src/botlib/botlib.h | 2 +-
src/botlib/l_log.h | 4 ++--
src/botlib/l_precomp.h | 4 ++--
src/botlib/l_script.h | 4 ++--
src/cgame/cg_local.h | 4 ++--
src/game/bg_lib.h | 2 +-
src/game/g_local.h | 6 +++---
src/master/common.h | 2 +-
src/qcommon/q_shared.h | 12 ++++++------
src/qcommon/qcommon.h | 10 +++++-----
src/renderer/tr_public.h | 4 ++--
src/server/server.h | 2 +-
src/ui/ui_shared.h | 4 ++--
14 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/src/botlib/be_aas_main.h b/src/botlib/be_aas_main.h
index 9f97818..e6b9eec 100644
--- a/src/botlib/be_aas_main.h
+++ b/src/botlib/be_aas_main.h
@@ -35,7 +35,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
extern aas_t aasworld;
//AAS error message
-void QDECL AAS_Error(char *fmt, ...);
+void QDECL AAS_Error(char *fmt, ...) __attribute__((format(printf, 1, 2)));
//set AAS initialized
void AAS_SetInitialized(void);
//setup AAS with the given number of entities and clients
diff --git a/src/botlib/botlib.h b/src/botlib/botlib.h
index 6c5147d..1ae8442 100644
--- a/src/botlib/botlib.h
+++ b/src/botlib/botlib.h
@@ -170,7 +170,7 @@ typedef struct bot_entitystate_s
typedef struct botlib_import_s
{
//print messages from the bot library
- void (QDECL *Print)(int type, char *fmt, ...);
+ void (QDECL *Print)(int type, char *fmt, ...) __attribute__((format(printf, 2, 3)));
//trace a bbox through the world
void (*Trace)(bsp_trace_t *trace, vec3_t start, vec3_t mins, vec3_t maxs, vec3_t end, int passent, int contentmask);
//trace a bbox against a specific entity
diff --git a/src/botlib/l_log.h b/src/botlib/l_log.h
index 91cbd95..154f981 100644
--- a/src/botlib/l_log.h
+++ b/src/botlib/l_log.h
@@ -37,9 +37,9 @@ void Log_Close(void);
//close log file if present
void Log_Shutdown(void);
//write to the current opened log file
-void QDECL Log_Write(char *fmt, ...);
+void QDECL Log_Write(char *fmt, ...) __attribute__((format(printf, 1, 2)));
//write to the current opened log file with a time stamp
-void QDECL Log_WriteTimeStamped(char *fmt, ...);
+void QDECL Log_WriteTimeStamped(char *fmt, ...) __attribute__((format(printf, 1, 2)));
//returns a pointer to the log file
FILE *Log_FilePointer(void);
//flush log file
diff --git a/src/botlib/l_precomp.h b/src/botlib/l_precomp.h
index b61125b..929db61 100644
--- a/src/botlib/l_precomp.h
+++ b/src/botlib/l_precomp.h
@@ -153,9 +153,9 @@ source_t *LoadSourceMemory(char *ptr, int length, char *name);
//free the given source
void FreeSource(source_t *source);
//print a source error
-void QDECL SourceError(source_t *source, char *str, ...);
+void QDECL SourceError(source_t *source, char *str, ...) __attribute__((format(printf, 2, 3)));
//print a source warning
-void QDECL SourceWarning(source_t *source, char *str, ...);
+void QDECL SourceWarning(source_t *source, char *str, ...) __attribute__((format(printf, 2, 3)));
#ifdef BSPC
// some of BSPC source does include game/q_shared.h and some does not
diff --git a/src/botlib/l_script.h b/src/botlib/l_script.h
index a779e62..a5cab5a 100644
--- a/src/botlib/l_script.h
+++ b/src/botlib/l_script.h
@@ -241,8 +241,8 @@ void FreeScript(script_t *script);
//set the base folder to load files from
void PS_SetBaseFolder(char *path);
//print a script error with filename and line number
-void QDECL ScriptError(script_t *script, char *str, ...);
+void QDECL ScriptError(script_t *script, char *str, ...) __attribute__((format(printf, 2, 3)));
//print a script warning with filename and line number
-void QDECL ScriptWarning(script_t *script, char *str, ...);
+void QDECL ScriptWarning(script_t *script, char *str, ...) __attribute__((format(printf, 2, 3)));
diff --git a/src/cgame/cg_local.h b/src/cgame/cg_local.h
index 320e060..7673919 100644
--- a/src/cgame/cg_local.h
+++ b/src/cgame/cg_local.h
@@ -1505,8 +1505,8 @@ extern vmCvar_t cg_debugRandom;
const char *CG_ConfigString( int index );
const char *CG_Argv( int arg );
-void QDECL CG_Printf( const char *msg, ... );
-void QDECL CG_Error( const char *msg, ... );
+void QDECL CG_Printf( const char *msg, ... ) __attribute__((format(printf, 1, 2)));
+void QDECL CG_Error( const char *msg, ... ) __attribute__((format(printf, 1, 2)));
void CG_StartMusic( void );
int CG_PlayerCount( void );
diff --git a/src/game/bg_lib.h b/src/game/bg_lib.h
index 021ebc3..01579a5 100644
--- a/src/game/bg_lib.h
+++ b/src/game/bg_lib.h
@@ -80,7 +80,7 @@ int _atoi( const char **stringPtr );
int vsprintf( char *buffer, const char *fmt, va_list argptr );
-int sscanf( const char *buffer, const char *fmt, ... );
+int sscanf( const char *buffer, const char *fmt, ... ) __attribute__((format(scanf, 2, 3)));
// Memory functions
void *memmove( void *dest, const void *src, size_t count );
diff --git a/src/game/g_local.h b/src/game/g_local.h
index 82f294b..830d5af 100644
--- a/src/game/g_local.h
+++ b/src/game/g_local.h
@@ -881,10 +881,10 @@ void MoveClientToIntermission( gentity_t *client );
void CalculateRanks( void );
void FindIntermissionPoint( void );
void G_RunThink( gentity_t *ent );
-void QDECL G_LogPrintf( const char *fmt, ... );
+void QDECL G_LogPrintf( const char *fmt, ... ) __attribute__((format(printf, 1, 2)));
void SendScoreboardMessageToAllClients( void );
-void QDECL G_Printf( const char *fmt, ... );
-void QDECL G_Error( const char *fmt, ... );
+void QDECL G_Printf( const char *fmt, ... ) __attribute__((format(printf, 1, 2)));
+void QDECL G_Error( const char *fmt, ... ) __attribute__((format(printf, 1, 2)));
//
// g_client.c
diff --git a/src/master/common.h b/src/master/common.h
index 47c29a9..c237bcd 100644
--- a/src/master/common.h
+++ b/src/master/common.h
@@ -82,7 +82,7 @@ extern char peer_address [128];
#endif
// Print a message to screen, depending on its verbose level
-int MsgPrint (msg_level_t msg_level, const char* format, ...);
+int MsgPrint (msg_level_t msg_level, const char* format, ...) __attribute__((format(printf, 2, 3)));
void RecordClientStat( const char *address, const char *version, const char *renderer );
void RecordGameStat( const char *address, const char *dataText );
diff --git a/src/qcommon/q_shared.h b/src/qcommon/q_shared.h
index 83f5789..8c83a5f 100644
--- a/src/qcommon/q_shared.h
+++ b/src/qcommon/q_shared.h
@@ -636,8 +636,8 @@ int COM_GetCurrentParseLine( void );
char *COM_Parse( char **data_p );
char *COM_ParseExt( char **data_p, qboolean allowLineBreak );
int COM_Compress( char *data_p );
-void COM_ParseError( char *format, ... );
-void COM_ParseWarning( char *format, ... );
+void COM_ParseError( char *format, ... ) __attribute__((format(printf, 1, 2)));
+void COM_ParseWarning( char *format, ... ) __attribute__((format(printf, 1, 2)));
//int COM_ParseInfos( char *buf, int max, char infos[][MAX_INFO_STRING] );
#define MAX_TOKENLENGTH 1024
@@ -671,7 +671,7 @@ void Parse1DMatrix (char **buf_p, int x, float *m);
void Parse2DMatrix (char **buf_p, int y, int x, float *m);
void Parse3DMatrix (char **buf_p, int z, int y, int x, float *m);
-void QDECL Com_sprintf (char *dest, int size, const char *fmt, ...);
+void QDECL Com_sprintf (char *dest, int size, const char *fmt, ...) __attribute__((format(printf, 3, 4)));
char *Com_SkipTokens( char *s, int numTokens, char *sep );
char *Com_SkipCharset( char *s, char *sep );
@@ -743,7 +743,7 @@ float LittleFloat (const float *l);
void Swap_Init (void);
*/
-char * QDECL va(char *format, ...);
+char * QDECL va(char *format, ...) __attribute__((format(printf, 1, 2)));
#define TRUNCATE_LENGTH 64
void Com_TruncateLongString( char *buffer, const char *s );
@@ -762,8 +762,8 @@ qboolean Info_Validate( const char *s );
void Info_NextPair( const char **s, char *key, char *value );
// this is only here so the functions in q_shared.c and bg_*.c can link
-void QDECL Com_Error( int level, const char *error, ... );
-void QDECL Com_Printf( const char *msg, ... );
+void QDECL Com_Error( int level, const char *error, ... ) __attribute__((format(printf, 2, 3)));
+void QDECL Com_Printf( const char *msg, ... ) __attribute__((format(printf, 1, 2)));
/*
diff --git a/src/qcommon/qcommon.h b/src/qcommon/qcommon.h
index 7b2fb8a..5db3699 100644
--- a/src/qcommon/qcommon.h
+++ b/src/qcommon/qcommon.h
@@ -162,7 +162,7 @@ void NET_Restart( void );
void NET_Config( qboolean enableNetworking );
void NET_SendPacket (netsrc_t sock, int length, const void *data, netadr_t to);
-void QDECL NET_OutOfBandPrint( netsrc_t net_socket, netadr_t adr, const char *format, ...);
+void QDECL NET_OutOfBandPrint( netsrc_t net_socket, netadr_t adr, const char *format, ...) __attribute__((format(printf, 3, 4)));
void QDECL NET_OutOfBandData( netsrc_t sock, netadr_t adr, byte *format, int len );
qboolean NET_CompareAdr (netadr_t a, netadr_t b);
@@ -719,9 +719,9 @@ void Info_Print( const char *s );
void Com_BeginRedirect (char *buffer, int buffersize, void (*flush)(char *));
void Com_EndRedirect( void );
-void QDECL Com_Printf( const char *fmt, ... );
-void QDECL Com_DPrintf( const char *fmt, ... );
-void QDECL Com_Error( int code, const char *fmt, ... );
+void QDECL Com_Printf( const char *fmt, ... ) __attribute__((format(printf, 1, 2)));
+void QDECL Com_DPrintf( const char *fmt, ... ) __attribute__((format(printf, 1, 2)));
+void QDECL Com_Error( int code, const char *fmt, ... ) __attribute__((format(printf, 2, 3)));
void Com_Quit_f( void );
int Com_EventLoop( void );
int Com_Milliseconds( void ); // will be journaled properly
@@ -978,7 +978,7 @@ void *Sys_GetBotLibAPI( void *parms );
char *Sys_GetCurrentUser( void );
-void QDECL Sys_Error( const char *error, ...);
+void QDECL Sys_Error( const char *error, ...) __attribute__((format(printf, 1, 2)));
void Sys_Quit (void);
char *Sys_GetClipboardData( void ); // note that this isn't journaled...
diff --git a/src/renderer/tr_public.h b/src/renderer/tr_public.h
index e4e4d04..8f3bb78 100644
--- a/src/renderer/tr_public.h
+++ b/src/renderer/tr_public.h
@@ -107,10 +107,10 @@ typedef struct {
//
typedef struct {
// print message on the local console
- void (QDECL *Printf)( int printLevel, const char *fmt, ...);
+ void (QDECL *Printf)( int printLevel, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
// abort the game
- void (QDECL *Error)( int errorLevel, const char *fmt, ...);
+ void (QDECL *Error)( int errorLevel, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
// milliseconds should only be used for profiling, never
// for anything game related. Get time from the refdef
diff --git a/src/server/server.h b/src/server/server.h
index 8eb4355..d1e764e 100644
--- a/src/server/server.h
+++ b/src/server/server.h
@@ -251,7 +251,7 @@ extern cvar_t *sv_lanForceRate;
// sv_main.c
//
void SV_FinalMessage (char *message);
-void QDECL SV_SendServerCommand( client_t *cl, const char *fmt, ...);
+void QDECL SV_SendServerCommand( client_t *cl, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
void SV_AddOperatorCommands (void);
diff --git a/src/ui/ui_shared.h b/src/ui/ui_shared.h
index 09de834..737900d 100644
--- a/src/ui/ui_shared.h
+++ b/src/ui/ui_shared.h
@@ -352,8 +352,8 @@ typedef struct {
void (*getBindingBuf)( int keynum, char *buf, int buflen );
void (*setBinding)( int keynum, const char *binding );
void (*executeText)(int exec_when, const char *text );
- void (*Error)(int level, const char *error, ...);
- void (*Print)(const char *msg, ...);
+ void (*Error)(int level, const char *error, ...) __attribute__((format(printf, 2, 3)));
+ void (*Print)(const char *msg, ...) __attribute__((format(printf, 1, 2)));
void (*Pause)(qboolean b);
int (*ownerDrawWidth)(int ownerDraw, float scale);
sfxHandle_t (*registerSound)(const char *name, qboolean compressed);
--- End Message ---