On Tue, 14 Mar 2017 at 08:30:36 +0000, Simon McVittie wrote:
> On Tue, 14 Mar 2017 at 04:59:15 +0100, Daniel Gibson wrote:
> > earlier today ioquake3 fixed a vulnerability that, as far as I understand,
> > could let malicious multiplayer servers execute code on connecting clients.
> > It affects all prior versions of ioquake3 (and I think also original Quake
> > 3).
> > Details:
> > https://ioquake3.org/2017/03/13/important-security-update-please-update-ioquake3-immediately/
Hi security team,
I would like to propose this debdiff for stable (assuming that testing it
later today goes as expected - I don't have access to a jessie system
that can run games right now).
The other change I made in unstable (putting the auto-downloading option
for Quake III Arena behind an "are you sure?" prompt) is not straightforward,
and only affects code without security support (quake3 but not openarena),
so I have omitted it from this version.
S
diffstat for ioquake3-1.36+u20140802+gca9eebb ioquake3-1.36+u20140802+gca9eebb
.gitignore | 6
changelog | 19 ++
gbp.conf | 2
patches/security/Don-t-load-.pk3s-as-.dlls-and-don-t-load-user-config-file.patch | 76 ++++++++++
patches/security/Don-t-open-.pk3-files-as-OpenAL-drivers.patch | 33 ++++
patches/security/Merge-some-file-writing-extension-checks-from-OpenJK.patch | 50 ++++++
patches/series | 3
7 files changed, 188 insertions(+), 1 deletion(-)
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/changelog ioquake3-1.36+u20140802+gca9eebb/debian/changelog
--- ioquake3-1.36+u20140802+gca9eebb/debian/changelog 2014-09-29 19:05:56.000000000 +0100
+++ ioquake3-1.36+u20140802+gca9eebb/debian/changelog 2017-03-14 11:55:27.000000000 +0000
@@ -1,3 +1,22 @@
+ioquake3 (1.36+u20140802+gca9eebb-2+deb8u1) jessie-security; urgency=high
+
+ * d/gbp.conf: switch branch to debian/jessie
+ * d/patches: Add patches from upstream fixing security vulnerabilities
+ - refuse to load potentially auto-downloadable .pk3 files as
+ ioquake3 renderers, ioquake3 game code, libcurl, or OpenAL drivers
+ (mitigation: auto-downloading is off by default, and in Debian
+ we do not dlopen libcurl anyway)
+ - refuse to load default configuration file names from a .pk3 file
+ - protect cl_renderer, cl_curllib, s_aldriver configuration variables so
+ game code cannot set them
+ - refuse to overwrite files other than *.txt with the dump console
+ command
+ - refuse to overwrite files other than *.cfg with the writeconfig
+ console command
+ (Closes: #857699)
+
+ -- Simon McVittie <s...@debian.org> Tue, 14 Mar 2017 11:54:04 +0000
+
ioquake3 (1.36+u20140802+gca9eebb-2) unstable; urgency=medium
* Build-depend on libjpeg-dev for jpeg-turbo transition
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/gbp.conf ioquake3-1.36+u20140802+gca9eebb/debian/gbp.conf
--- ioquake3-1.36+u20140802+gca9eebb/debian/gbp.conf 2014-09-29 19:05:56.000000000 +0100
+++ ioquake3-1.36+u20140802+gca9eebb/debian/gbp.conf 2017-03-14 11:55:27.000000000 +0000
@@ -1,5 +1,5 @@
[DEFAULT]
-debian-branch = master
+debian-branch = debian/jessie
upstream-branch = upstream
pristine-tar = True
patch-numbers = False
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/.gitignore ioquake3-1.36+u20140802+gca9eebb/debian/.gitignore
--- ioquake3-1.36+u20140802+gca9eebb/debian/.gitignore 1970-01-01 01:00:00.000000000 +0100
+++ ioquake3-1.36+u20140802+gca9eebb/debian/.gitignore 2017-03-14 11:55:27.000000000 +0000
@@ -0,0 +1,6 @@
+files
+*.debhelper.log
+*.substvars
+ioquake3/
+ioquake3-dbg/
+ioquake3-server/
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-load-.pk3s-as-.dlls-and-don-t-load-user-config-file.patch ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-load-.pk3s-as-.dlls-and-don-t-load-user-config-file.patch
--- ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-load-.pk3s-as-.dlls-and-don-t-load-user-config-file.patch 1970-01-01 01:00:00.000000000 +0100
+++ ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-load-.pk3s-as-.dlls-and-don-t-load-user-config-file.patch 2017-03-14 11:55:27.000000000 +0000
@@ -0,0 +1,76 @@
+From: SmileTheory <smilethe...@gmail.com>
+Date: Mon, 13 Mar 2017 14:14:00 -0700
+Subject: Don't load .pk3s as .dlls,
+ and don't load user config files from .pk3s.
+
+Origin: upstream, 1.37, commit:376267d534476a875d8b9228149c4ee18b74a4fd
+Bug-Debian: https://bugs.debian.org/857699
+---
+ code/client/cl_main.c | 4 ++--
+ code/qcommon/files.c | 6 ++++++
+ code/sys/sys_main.c | 7 +++++++
+ 3 files changed, 15 insertions(+), 2 deletions(-)
+
+diff --git a/code/client/cl_main.c b/code/client/cl_main.c
+index 2fa38af..026ebd0 100644
+--- a/code/client/cl_main.c
++++ b/code/client/cl_main.c
+@@ -3179,7 +3179,7 @@ void CL_InitRef( void ) {
+ Com_Printf( "----- Initializing Renderer ----\n" );
+
+ #ifdef USE_RENDERER_DLOPEN
+- cl_renderer = Cvar_Get("cl_renderer", "opengl1", CVAR_ARCHIVE | CVAR_LATCH);
++ cl_renderer = Cvar_Get("cl_renderer", "opengl1", CVAR_ARCHIVE | CVAR_LATCH | CVAR_PROTECTED);
+
+ Com_sprintf(dllName, sizeof(dllName), "renderer_%s_" ARCH_STRING DLL_EXT, cl_renderer->string);
+
+@@ -3479,7 +3479,7 @@ void CL_Init( void ) {
+
+ cl_allowDownload = Cvar_Get ("cl_allowDownload", "0", CVAR_ARCHIVE);
+ #ifdef USE_CURL_DLOPEN
+- cl_cURLLib = Cvar_Get("cl_cURLLib", DEFAULT_CURL_LIB, CVAR_ARCHIVE);
++ cl_cURLLib = Cvar_Get("cl_cURLLib", DEFAULT_CURL_LIB, CVAR_ARCHIVE | CVAR_PROTECTED);
+ #endif
+
+ cl_conXOffset = Cvar_Get ("cl_conXOffset", "0", 0);
+diff --git a/code/qcommon/files.c b/code/qcommon/files.c
+index b56411c..e82120e 100644
+--- a/code/qcommon/files.c
++++ b/code/qcommon/files.c
+@@ -1344,12 +1344,18 @@ long FS_FOpenFileRead(const char *filename, fileHandle_t *file, qboolean uniqueF
+ {
+ searchpath_t *search;
+ long len;
++ qboolean isLocalConfig;
+
+ if(!fs_searchpaths)
+ Com_Error(ERR_FATAL, "Filesystem call made without initialization");
+
++ isLocalConfig = !strcmp(filename, "autoexec.cfg") || !strcmp(filename, Q3CONFIG_CFG);
+ for(search = fs_searchpaths; search; search = search->next)
+ {
++ // autoexec.cfg and q3config.cfg can only be loaded outside of pk3 files.
++ if (isLocalConfig && search->pack)
++ continue;
++
+ len = FS_FOpenFileReadDir(filename, search, file, uniqueFILE, qfalse);
+
+ if(file == NULL)
+diff --git a/code/sys/sys_main.c b/code/sys/sys_main.c
+index 391ec0b..ee4f911 100644
+--- a/code/sys/sys_main.c
++++ b/code/sys/sys_main.c
+@@ -434,6 +434,13 @@ void *Sys_LoadDll(const char *name, qboolean useSystemLib)
+ {
+ void *dllhandle;
+
++ // Don't load any DLLs that end with the pk3 extension
++ if (COM_CompareExtension(name, ".pk3"))
++ {
++ Com_Printf("Rejecting DLL named \"%s\"", name);
++ return NULL;
++ }
++
+ if(useSystemLib)
+ Com_Printf("Trying to load \"%s\"...\n", name);
+
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-open-.pk3-files-as-OpenAL-drivers.patch ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-open-.pk3-files-as-OpenAL-drivers.patch
--- ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-open-.pk3-files-as-OpenAL-drivers.patch 1970-01-01 01:00:00.000000000 +0100
+++ ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Don-t-open-.pk3-files-as-OpenAL-drivers.patch 2017-03-14 11:55:27.000000000 +0000
@@ -0,0 +1,33 @@
+From: SmileTheory <smilethe...@gmail.com>
+Date: Mon, 13 Mar 2017 20:28:37 -0700
+Subject: Don't open .pk3 files as OpenAL drivers.
+
+Origin: upstream, 1.37, commit:f61fe5f6a0419ef4a88d46a128052f2e8352e85d
+Bug-Debian: https://bugs.debian.org/857699
+---
+ code/client/snd_openal.c | 8 +++++++-
+ 1 file changed, 7 insertions(+), 1 deletion(-)
+
+diff --git a/code/client/snd_openal.c b/code/client/snd_openal.c
+index 36f5deb..f828fc5 100644
+--- a/code/client/snd_openal.c
++++ b/code/client/snd_openal.c
+@@ -2512,11 +2512,17 @@ qboolean S_AL_Init( soundInterface_t *si )
+ s_alRolloff = Cvar_Get( "s_alRolloff", "2", CVAR_CHEAT);
+ s_alGraceDistance = Cvar_Get("s_alGraceDistance", "512", CVAR_CHEAT);
+
+- s_alDriver = Cvar_Get( "s_alDriver", ALDRIVER_DEFAULT, CVAR_ARCHIVE | CVAR_LATCH );
++ s_alDriver = Cvar_Get( "s_alDriver", ALDRIVER_DEFAULT, CVAR_ARCHIVE | CVAR_LATCH | CVAR_PROTECTED );
+
+ s_alInputDevice = Cvar_Get( "s_alInputDevice", "", CVAR_ARCHIVE | CVAR_LATCH );
+ s_alDevice = Cvar_Get("s_alDevice", "", CVAR_ARCHIVE | CVAR_LATCH);
+
++ if ( COM_CompareExtension( s_alDriver->string, ".pk3" ) )
++ {
++ Com_Printf( "Rejecting DLL named \"%s\"", s_alDriver->string );
++ return qfalse;
++ }
++
+ // Load QAL
+ if( !QAL_Init( s_alDriver->string ) )
+ {
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Merge-some-file-writing-extension-checks-from-OpenJK.patch ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Merge-some-file-writing-extension-checks-from-OpenJK.patch
--- ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Merge-some-file-writing-extension-checks-from-OpenJK.patch 1970-01-01 01:00:00.000000000 +0100
+++ ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Merge-some-file-writing-extension-checks-from-OpenJK.patch 2017-03-14 11:55:27.000000000 +0000
@@ -0,0 +1,50 @@
+From: SmileTheory <smilethe...@gmail.com>
+Date: Mon, 13 Mar 2017 20:44:47 -0700
+Subject: Merge some file writing extension checks from OpenJK.
+
+Thanks Ensiform.
+https://github.com/JACoders/OpenJK/commit/05928a57f9e4aae15a3bd0
+https://github.com/JACoders/OpenJK/commit/ef124fd0fc48af164581176
+
+Origin: upstream, 1.37, commit:b173ac05993f634a42be3d3535e1b158de0c3372
+Bug-Debian: https://bugs.debian.org/857699
+---
+ code/client/cl_console.c | 6 ++++++
+ code/qcommon/common.c | 7 +++++++
+ 2 files changed, 13 insertions(+)
+
+diff --git a/code/client/cl_console.c b/code/client/cl_console.c
+index a3d4d32..7de318d 100644
+--- a/code/client/cl_console.c
++++ b/code/client/cl_console.c
+@@ -191,6 +191,12 @@ void Con_Dump_f (void)
+ Q_strncpyz( filename, Cmd_Argv( 1 ), sizeof( filename ) );
+ COM_DefaultExtension( filename, sizeof( filename ), ".txt" );
+
++ if (!COM_CompareExtension(filename, ".txt"))
++ {
++ Com_Printf("Con_Dump_f: Only the \".txt\" extension is supported by this command!\n");
++ return;
++ }
++
+ f = FS_FOpenFileWrite( filename );
+ if (!f)
+ {
+diff --git a/code/qcommon/common.c b/code/qcommon/common.c
+index 36cb4ce..309be1f 100644
+--- a/code/qcommon/common.c
++++ b/code/qcommon/common.c
+@@ -2961,6 +2961,13 @@ void Com_WriteConfig_f( void ) {
+ return;
+ }
+
++
++ if (!COM_CompareExtension(filename, ".cfg"))
++ {
++ Com_Printf("Com_WriteConfig_f: Only the \".cfg\" extension is supported by this command!\n");
++ return;
++ }
++
+ Q_strncpyz( filename, Cmd_Argv(1), sizeof( filename ) );
+ COM_DefaultExtension( filename, sizeof( filename ), ".cfg" );
+ Com_Printf( "Writing %s.\n", filename );
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/patches/series ioquake3-1.36+u20140802+gca9eebb/debian/patches/series
--- ioquake3-1.36+u20140802+gca9eebb/debian/patches/series 2014-09-29 19:05:56.000000000 +0100
+++ ioquake3-1.36+u20140802+gca9eebb/debian/patches/series 2017-03-14 11:55:27.000000000 +0000
@@ -3,3 +3,6 @@
Let-servers-set-sv_fps-too.patch
Run-in-a-window-by-default-on-new-installations.patch
Add-support-for-the-GNU-Hurd-architecture.patch
+security/Don-t-load-.pk3s-as-.dlls-and-don-t-load-user-config-file.patch
+security/Don-t-open-.pk3-files-as-OpenAL-drivers.patch
+security/Merge-some-file-writing-extension-checks-from-OpenJK.patch
