Bug#686648: ioquake3: consider disallowing auto-downloading in wheezy
Package: ioquake3
Version: 1.36+svn2287-1
Severity: important
Tags: patch
X-Debbugs-Cc: debian-release@lists.debian.org
X-Debbugs-Cc: debian-devel-games@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 <smcv@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 <fuddl@debian.org>,
Simon McVittie <smcv@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 <smcv@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 <smcv@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 <smcv@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 <smcv@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
Reply to: