Package: ioquake3 Version: 1.36+svn2287-1 Severity: important Tags: patch X-Debbugs-Cc: debian-rele...@lists.debian.org X-Debbugs-Cc: debian-devel-ga...@lists.debian.org
I am considering removing the cl_allowDownload option from the ioquake3 client, effectively forcing its value to "disabled" (further details below). The effect of this option is: * if disabled (or patched out), joining "modded" game servers will require users to download and install any "mods" active on that server manually * if enabled, "mods" are automatically downloaded; if certain security flaws exist in ioquake3, a malicious server operator or a man-in-the-middle could exercise those flaws (worst-case: arbitrary code execution) by encouraging users to join a game server This is basically a trade-off between convenience and mitigating security vulnerabilities. I say "mitigating" because a user could always install a malicious mod to ~/.q3a or ~/.openarena manually, with the same result as if they had auto-downloaded it. I am not aware of any current vulnerabilities that could be exploited in this way, but see below for a list of past vulnerabilities that would have been mitigated by this change. Games team: what are your thoughts about this? Should we give users the freedom to shoot themselves in the foot, or patch this feature out? Should we reinstate the feature in unstable after wheezy releases, or leave it out permanently? Release team: would you consider a freeze exception for this? I attach draft patches (I'd replace nnnnnn with this bug number and UNRELEASED with unstable, obviously). Only the ioquake3 one is strictly necessary, but it would leave a useless and misleading menu option in openarena, so I would prefer to patch openarena too. The next "obvious" revision numbers (ioquake3 1.36+svn2287-2, openarena 0.8.8-6) are already in use in experimental, so if I upload these, I'm going to version them like a stable update. Let me know if you would prefer me to use -X+wheezyY for the revision numbers rather than -X+deb70+Y, or something else entirely. S ---- Further explanation: The ioquake3 engine is used in openarena (main/games) and quake3 (contrib/games). When used as a network client, it has the option to auto-download required data from the game server, or (as one of the ioquake3 enhancements to the Quake III Arena engine) from a HTTP or FTP server nominated by the server administrator. By design, auto-downloaded packages are not signed or authenticated (server administrators can add arbitrary "mods"). As well as "safe" data (maps, 3D models etc.), auto-downloaded packages can include executable bytecode (cgame.qvm, ui.qvm), which will be run by the client using a JIT or interpreter. The JIT/interpreter acts as a simple sandbox, and known vulnerabilities in it have been treated as security issues and fixed. To the best of my knowledge, there has not been a systematic audit. Unfortunately, it is not currently possible to auto-download "safe" files (maps, models, textures, music etc.) but reject executable bytecode. I hope to add that feature in time for Debian 8, and make it the default. During squeeze updates to tremulous (which uses a fork of ioquake3), I patched out auto-downloading support. I am now considering doing the same to ioquake3 itself before wheezy is released: this would mean that any vulnerabilities discovered in the bytecode JIT/interpreter would not affect wheezy. However, this would remove an apparently-intentional feature, making it harder for Debian users to join "modded" servers. In Quake III Arena (quake3, contrib/games) enabling client-side auto-downloading requires console commands; in OpenArena (openarena, main/games) the feature can be enabled through the GUI. In both cases it is off by default. Server administrators and gaming communities frequently encourage users to switch on this feature, apparently without considering its security implications. Here are some past Quake III Arena CVEs and whether this change would have mitigated them: affects impact mitigated by this? CVE-2001-1289 server DoS no CVE-2005-0430 server DoS no CVE-2005-0983 client DoS no CVE-2006-2082 server info disclos no CVE-2006-2236 client code exec no CVE-2007-2785 client code exec yes CVE-2006-3324 client file write yes CVE-2006-3325 client code exec? partially? CVE-2006-3400 client code exec? no CVE-2006-3401 client code exec yes? CVE-2011-1412 client code exec no CVE-2011-2764 client code exec yes CVE-2012-3345 both file write no -- System Information: Debian Release: wheezy/sid APT prefers testing-proposed-updates APT policy: (500, 'testing-proposed-updates'), (500, 'unstable'), (500, 'testing'), (1, 'experimental') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 3.2.0-3-amd64 (SMP w/4 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Versions of packages ioquake3 depends on: ii libc6 2.13-35 ii libcurl3-gnutls 7.27.0-1 ii libgl1-mesa-glx [libgl1] 8.0.4-2 ii libjpeg8 8d-1 ii libogg0 1.3.0-4 ii libopenal1 1:1.14-4 ii libsdl1.2debian 1.2.15-5 ii libspeex1 1.2~rc1-6 ii libspeexdsp1 1.2~rc1-6 ii libvorbis0a 1.3.2-1.3 ii libvorbisfile3 1.3.2-1.3 ii zlib1g 1:1.2.7.dfsg-13 Versions of packages ioquake3 recommends: ii x11-utils 7.7~1 ii zenity 3.4.0-2 ioquake3 suggests no packages. Versions of packages ioquake3 is related to: ii libgl1-mesa-dri 8.0.4-2 -- no debconf information
diff --git a/debian/changelog b/debian/changelog index ee5a41c..b53a2c2 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +ioquake3 (1.36+svn2287-1+d70+1) UNRELEASED; urgency=low + + * Turn off client-side auto-downloading as a precaution against possible + vulnerabilities in execution of untrusted bytecode (Closes: #nnnnnn) + * Disable cURL support and do not build-depend on it; it is only + useful if client-side auto-downloading is allowed + + -- Simon McVittie <s...@debian.org> Tue, 04 Sep 2012 08:57:20 +0100 + ioquake3 (1.36+svn2287-1) unstable; urgency=low * New upstream snapshot diff --git a/debian/control b/debian/control index 0b33ae8..32caccd 100644 --- a/debian/control +++ b/debian/control @@ -6,7 +6,6 @@ Uploaders: Bruno "Fuddl" Kleinert <fu...@debian.org>, Simon McVittie <s...@debian.org> Build-Depends: debhelper (>= 9), dpkg-dev (>= 1.16.1), - libcurl4-gnutls-dev, libjpeg8-dev, libopenal-dev, libsdl1.2-dev, diff --git a/debian/patches/0010-permanently-disable-auto-downloading.patch b/debian/patches/0010-permanently-disable-auto-downloading.patch new file mode 100644 index 0000000..b686cef --- /dev/null +++ b/debian/patches/0010-permanently-disable-auto-downloading.patch @@ -0,0 +1,62 @@ +From: Simon McVittie <s...@debian.org> +Date: 2012-09-04 +Subject: permanently disable auto-downloading + +Turn off client-side auto-downloading as a precaution against possible +vulnerabilities in execution of untrusted bytecode. + +Origin: vendor, Debian +Bug-Debian: http://bugs.debian.org/nnnnnn +Forwarded: not-needed, Debian-specific +--- + code/client/cl_main.c | 8 ++++++-- + 1 file changed, 6 insertions(+), 2 deletions(-) +diff --git a/code/client/cl_main.c b/code/client/cl_main.c +index b506dbd..f2a256e 100644 +--- a/code/client/cl_main.c ++++ b/code/client/cl_main.c +@@ -2151,6 +2151,7 @@ A download completed or failed + */ + void CL_NextDownload(void) + { ++#if 0 + char *s; + char *remoteName, *localName; + qboolean useCURL = qfalse; +@@ -2239,6 +2240,7 @@ void CL_NextDownload(void) + + return; + } ++#endif + + CL_DownloadsComplete(); + } +@@ -2254,7 +2256,7 @@ and determine if we need to download them + void CL_InitDownloads(void) { + char missingfiles[1024]; + +- if ( !(cl_allowDownload->integer & DLF_ENABLE) ) ++ if ( 1 ) + { + // autodownload is disabled on the client + // but it's possible that some referenced files on the server are missing +@@ -2264,9 +2266,10 @@ 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 ); + } + } ++#if 0 + else if ( FS_ComparePaks( clc.downloadList, sizeof( clc.downloadList ) , qtrue ) ) { + + Com_Printf("Need paks: %s\n", clc.downloadList ); +@@ -2283,6 +2286,7 @@ void CL_InitDownloads(void) { + } + + } ++#endif + + CL_DownloadsComplete(); + } diff --git a/debian/patches/series b/debian/patches/series index 57083f4..2404fd4 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -7,3 +7,4 @@ 0007-Let-servers-set-sv_fps-too.patch 0008-Only-emit-i486-instructions-on-x86-but-optimize-for-.patch 0009-Run-in-a-window-by-default-on-new-installations.patch +0010-permanently-disable-auto-downloading.patch diff --git a/debian/rules b/debian/rules index 7bedea7..1f55d1a 100755 --- a/debian/rules +++ b/debian/rules @@ -27,7 +27,7 @@ override_dh_auto_build: BD=build \ V=1 \ USE_CODEC_VORBIS=1 \ - USE_CURL=1 \ + USE_CURL=0 \ USE_CURL_DLOPEN=0 \ USE_OPENAL=1 \ USE_OPENAL_DLOPEN=0 \
diff --git a/debian/changelog b/debian/changelog index 58ad24a..8e29f74 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,13 @@ +openarena (0.8.8-5+deb70+1) UNRELEASED; urgency=low + + * Remove auto-downloading from first-connect menu. In ioquake3 + 1.36+svn2287-1+d70+1 the option no longer does anything. + * Remove auto-downloading from Preferences menu and replace it with + a reference to a page on the Debian wiki which will explain why. + (See: #nnnnnn) + + -- Simon McVittie <s...@debian.org> Tue, 04 Sep 2012 09:59:43 +0100 + openarena (0.8.8-5) unstable; urgency=low * Don't refuse to start a new openarena-server if there's a stale diff --git a/debian/patches/0041-Remove-auto-downloading-from-first-connect-menu.patch b/debian/patches/0041-Remove-auto-downloading-from-first-connect-menu.patch new file mode 100644 index 0000000..cda3096 --- /dev/null +++ b/debian/patches/0041-Remove-auto-downloading-from-first-connect-menu.patch @@ -0,0 +1,106 @@ +From 3cfe53f397933a4b6664ee2ceb9338d4d53bd303 Mon Sep 17 00:00:00 2001 +From: Simon McVittie <s...@debian.org> +Date: Tue, 4 Sep 2012 10:05:40 +0100 +Subject: [PATCH 1/2] Remove auto-downloading from first-connect menu + +In ioquake3 1.36+svn2287-1+d70+1 the option no longer does anything. + +Origin: vendor, Debian +Bug-Debian: http://bugs.debian.org/nnnnnn +Forwarded: not-needed, Debian-specific +--- + code/q3_ui/ui_firstconnect.c | 14 ++++++++++++++ + 1 file changed, 14 insertions(+) + +diff --git a/code/q3_ui/ui_firstconnect.c b/code/q3_ui/ui_firstconnect.c +index 5291aea..5ef6071 100644 +--- a/code/q3_ui/ui_firstconnect.c ++++ b/code/q3_ui/ui_firstconnect.c +@@ -39,7 +39,9 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + #define ID_NAME 10 + #define ID_RATE 11 + #define ID_DELAGHITSCAN 12 ++#if 0 + #define ID_ALLOWDOWNLOAD 13 ++#endif + + #define ID_GO 100 + #define ID_BACK 101 +@@ -85,7 +87,9 @@ typedef struct + + //Optional options + menuradiobutton_s delaghitscan; ++#if 0 + menuradiobutton_s allowdownload; ++#endif + } firstconnect_t; + + static firstconnect_t s_firstconnect; +@@ -188,6 +192,7 @@ static void FirstConnect_StatusBar_Delag( void* ptr ) { + UI_DrawString( 320, 440, "Reliable Instanthit weapons", UI_CENTER|UI_SMALLFONT, colorWhite ); + } + ++#if 0 + /* + ================= + FirstConnect_StatusBar_Download +@@ -196,6 +201,7 @@ FirstConnect_StatusBar_Download + static void FirstConnect_StatusBar_Download( void* ptr ) { + UI_DrawString( 320, 440, "Auto download missing maps and mods", UI_CENTER|UI_SMALLFONT, colorWhite ); + } ++#endif + + /* + ================= +@@ -250,10 +256,12 @@ static void FirstConnect_Event( void* ptr, int event ) + } + break; + ++#if 0 + case ID_ALLOWDOWNLOAD: + trap_Cvar_SetValue( "cl_allowDownload", s_firstconnect.allowdownload.curvalue ); + trap_Cvar_SetValue( "sv_allowDownload", s_firstconnect.allowdownload.curvalue ); + break; ++#endif + + case ID_DELAGHITSCAN: + trap_Cvar_SetValue( "g_delagHitscan", s_firstconnect.delaghitscan.curvalue ); +@@ -289,7 +297,9 @@ static void FirstConnect_SetMenuItems( void ) { + s_firstconnect.rate.curvalue = 4; + } + ++#if 0 + s_firstconnect.allowdownload.curvalue = trap_Cvar_VariableValue( "cl_allowDownload" ) != 0; ++#endif + s_firstconnect.delaghitscan.curvalue = trap_Cvar_VariableValue( "cg_delag" ) != 0; + } + +@@ -389,6 +399,7 @@ void FirstConnect_MenuInit( void ) + s_firstconnect.delaghitscan.generic.y = y; + s_firstconnect.delaghitscan.generic.statusbar = FirstConnect_StatusBar_Delag; + ++#if 0 + y += BIGCHAR_HEIGHT+2; + s_firstconnect.allowdownload.generic.type = MTYPE_RADIOBUTTON; + s_firstconnect.allowdownload.generic.name = "Automatic Downloading:"; +@@ -398,6 +409,7 @@ void FirstConnect_MenuInit( void ) + s_firstconnect.allowdownload.generic.x = 320; + s_firstconnect.allowdownload.generic.y = y; + s_firstconnect.allowdownload.generic.statusbar = FirstConnect_StatusBar_Download; ++#endif + + s_firstconnect.info.generic.type = MTYPE_TEXT; + s_firstconnect.info.generic.x = 320; +@@ -423,7 +435,9 @@ void FirstConnect_MenuInit( void ) + Menu_AddItem( &s_firstconnect.menu, &s_firstconnect.rate ); + + Menu_AddItem( &s_firstconnect.menu, &s_firstconnect.delaghitscan ); ++#if 0 + Menu_AddItem( &s_firstconnect.menu, &s_firstconnect.allowdownload ); ++#endif + + Menu_AddItem( &s_firstconnect.menu, &s_firstconnect.info ); + Menu_AddItem( &s_firstconnect.menu, &s_firstconnect.info2 ); +-- +1.7.10.4 + diff --git a/debian/patches/0042-Remove-auto-downloading-from-Preferences-menu.patch b/debian/patches/0042-Remove-auto-downloading-from-Preferences-menu.patch new file mode 100644 index 0000000..bbbb5bb --- /dev/null +++ b/debian/patches/0042-Remove-auto-downloading-from-Preferences-menu.patch @@ -0,0 +1,75 @@ +From e70fcc41e7acb1a02851547d3dce4c3b3c4aa00a Mon Sep 17 00:00:00 2001 +From: Simon McVittie <s...@debian.org> +Date: Tue, 4 Sep 2012 10:06:14 +0100 +Subject: [PATCH 2/2] Remove auto-downloading from Preferences menu + +In ioquake3 1.36+svn2287-1+d70+1 the option no longer does anything. +Replace it with a reference to a page on the Debian wiki which +will explain why. + +Origin: vendor, Debian +Bug-Debian: http://bugs.debian.org/nnnnnn +Forwarded: not-needed, Debian-specific + code/q3_ui/ui_preferences.c | 17 +++++++++++------ + 1 file changed, 11 insertions(+), 6 deletions(-) +diff --git a/code/q3_ui/ui_preferences.c b/code/q3_ui/ui_preferences.c +index 32b693f..bf6cb10 100644 +--- a/code/q3_ui/ui_preferences.c ++++ b/code/q3_ui/ui_preferences.c +@@ -90,7 +90,7 @@ typedef struct { + menuradiobutton_s forcemodel; + menulist_s drawteamoverlay; + menuradiobutton_s delaghitscan; +- menuradiobutton_s allowdownload; ++ menulist_s allowdownload; + menuradiobutton_s chatbeep; + menuradiobutton_s teamchatbeep; + menubitmap_s back; +@@ -109,6 +109,12 @@ static const char *teamoverlay_names[] = + NULL + }; + ++static const char *allowdownload_message[] = ++{ ++ "disabled, see http://deb.li/q3dl", ++ NULL ++}; ++ + static void Preferences_SetMenuItems( void ) { + s_preferences.crosshair.curvalue = (int)trap_Cvar_VariableValue( "cg_drawCrosshair" ) % NUM_CROSSHAIRS; + s_preferences.crosshairHealth.curvalue = trap_Cvar_VariableValue( "cg_crosshairHealth") != 0; +@@ -125,7 +131,7 @@ static void Preferences_SetMenuItems( void ) { + s_preferences.synceveryframe.curvalue = trap_Cvar_VariableValue( "r_finish" ) != 0; + s_preferences.forcemodel.curvalue = trap_Cvar_VariableValue( "cg_forcemodel" ) != 0; + s_preferences.drawteamoverlay.curvalue = Com_Clamp( 0, 3, trap_Cvar_VariableValue( "cg_drawTeamOverlay" ) ); +- s_preferences.allowdownload.curvalue = trap_Cvar_VariableValue( "cl_allowDownload" ) != 0; ++ s_preferences.allowdownload.curvalue = 0; + s_preferences.delaghitscan.curvalue = trap_Cvar_VariableValue( "cg_delag" ) != 0; + s_preferences.chatbeep.curvalue = trap_Cvar_VariableValue( "cg_chatBeep" ) != 0; + s_preferences.teamchatbeep.curvalue = trap_Cvar_VariableValue( "cg_teamChatBeep" ) != 0; +@@ -216,8 +222,6 @@ static void Preferences_Event( void* ptr, int notification ) { + break; + + case ID_ALLOWDOWNLOAD: +- trap_Cvar_SetValue( "cl_allowDownload", s_preferences.allowdownload.curvalue ); +- trap_Cvar_SetValue( "sv_allowDownload", s_preferences.allowdownload.curvalue ); + break; + + case ID_DELAGHITSCAN: +@@ -498,13 +502,14 @@ static void Preferences_MenuInit( void ) { + s_preferences.delaghitscan.generic.y = y; + + y += BIGCHAR_HEIGHT+2; +- s_preferences.allowdownload.generic.type = MTYPE_RADIOBUTTON; ++ s_preferences.allowdownload.generic.type = MTYPE_SPINCONTROL; + s_preferences.allowdownload.generic.name = "Automatic Downloading:"; +- s_preferences.allowdownload.generic.flags = QMF_PULSEIFFOCUS|QMF_SMALLFONT; ++ s_preferences.allowdownload.generic.flags = QMF_SMALLFONT; + s_preferences.allowdownload.generic.callback = Preferences_Event; + s_preferences.allowdownload.generic.id = ID_ALLOWDOWNLOAD; + s_preferences.allowdownload.generic.x = PREFERENCES_X_POS; + s_preferences.allowdownload.generic.y = y; ++ s_preferences.allowdownload.itemnames = allowdownload_message; + + y += BIGCHAR_HEIGHT+2; + s_preferences.chatbeep.generic.type = MTYPE_RADIOBUTTON; diff --git a/debian/patches/series b/debian/patches/series index 8af2664..ce25e1a 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,3 +1,5 @@ 0001-Use-a-cpp-macro-for-the-game-code-version-so-package.patch 0031-Fix-FTBFS-on-kFreeBSD.patch 0040-Add-OPENARENA_081_COMPATIBLE-define-for-network-comp.patch +0041-Remove-auto-downloading-from-first-connect-menu.patch +0042-Remove-auto-downloading-from-Preferences-menu.patch