[bug#33347,4/4] gnu: teeworlds: Update to 0.7.0 [fixes CVE-2018-18541].

Message ID 87k1lj1le4.fsf@gmail.com
State Accepted
Headers show
Series gnu: teeworlds: Update to 0.7.0 [fixes CVE-2018-18541]. | expand

Checks

Context Check Description
cbaines/applying patch success Successfully applied
cbaines/applying patch success Successfully applied
cbaines/applying patch success Successfully applied
cbaines/applying patch success
cbaines/Applying patch success Description

Commit Message

Alex Vong Nov. 11, 2018, 7:09 p.m. UTC

Comments

Leo Famulari Nov. 13, 2018, 4:53 p.m. UTC | #1
On Mon, Nov 12, 2018 at 03:09:39AM +0800, Alex Vong wrote:
>           (replace 'configure
>             (lambda* (#:key outputs #:allow-other-keys)
> +             (define (use-latest-json-parser file)
> +               (substitute* file
> +                 (("engine/external/json-parser/json\\.h")
> +                  "json-parser/json.h")
> +                 (("json_parse_ex\\(&JsonSettings, pFileData, aError\\);")
> +                  "json_parse_ex(&JsonSettings,
> +                                 pFileData,
> +                                 strlen(pFileData),
> +                                 aError);")))
> +

Please add a code comment explaining this.

> -    ;; FIXME: teeworlds bundles the sources of "pnglite", a two-file PNG
> -    ;; library without a build system.

These sorts of mini-libraries are designed to be copied and pasted into
host projects rather than packaged on their own. That's why they don't
include a build system. For example, many cryptographic primitive
implementations are distributed this way — that's why you never see a
package for 'SHA256'. Is there a particular reason we should unbundle
pnglite?
Alex Vong Nov. 14, 2018, 1:36 p.m. UTC | #2
Leo Famulari <leo@famulari.name> writes:

> On Mon, Nov 12, 2018 at 03:09:39AM +0800, Alex Vong wrote:
>>           (replace 'configure
>>             (lambda* (#:key outputs #:allow-other-keys)
>> +             (define (use-latest-json-parser file)
>> +               (substitute* file
>> +                 (("engine/external/json-parser/json\\.h")
>> +                  "json-parser/json.h")
>> +                 (("json_parse_ex\\(&JsonSettings, pFileData, aError\\);")
>> +                  "json_parse_ex(&JsonSettings,
>> +                                 pFileData,
>> +                                 strlen(pFileData),
>> +                                 aError);")))
>> +
>
> Please add a code comment explaining this.
>
OK

>> -    ;; FIXME: teeworlds bundles the sources of "pnglite", a two-file PNG
>> -    ;; library without a build system.
>
> These sorts of mini-libraries are designed to be copied and pasted into
> host projects rather than packaged on their own. That's why they don't
> include a build system. For example, many cryptographic primitive
> implementations are distributed this way — that's why you never see a
> package for 'SHA256'. Is there a particular reason we should unbundle
> pnglite?

Well, I though we have a policy to remove bundle dependencies in order
to avoid building the same library many times. Do we make exceptions for
shared libraries w/o a build system? (an exception I can think of is
gnulib)

Besides, the FIXME comment seems to suggest future readers to help
remove the bundled pnglite. Debian also removes the bundled pnglite in
teeworlds[0].

Thanks for all the feedback!

[0]: https://packages.debian.org/sid/teeworlds
Leo Famulari Nov. 14, 2018, 5:39 p.m. UTC | #3
On Wed, Nov 14, 2018 at 09:36:25PM +0800, Alex Vong wrote:
> Well, I though we have a policy to remove bundle dependencies in order
> to avoid building the same library many times. Do we make exceptions for
> shared libraries w/o a build system? (an exception I can think of is
> gnulib)

In general, yes, our policy is to unbundle things when practical.

But there are some commonly used software implementations of basic
functions (like base64, sha1 (most hash functions actually), et cetera)
that are specifically designed to be copied and pasted into the
application that will be using them.

You can usually tell this is the case because the thing will not have
any build system at all, like you suggest. Also because you find the
same copy-pasted code in almost every program you look at, like with
base64 and the hash functions.

> Besides, the FIXME comment seems to suggest future readers to help
> remove the bundled pnglite. Debian also removes the bundled pnglite in
> teeworlds[0].

Well, at a certain point it becomes a matter of taste, and the choice
should be made by the person doing the work — you! Either way is fine
for Guix :) The important thing is to get this Teeworlds fix pushed
without too much delay.

Patch

From 340a24167fe00a3ea62804bb97760b8ba3b2f6f8 Mon Sep 17 00:00:00 2001
From: Alex Vong <alexvong1995@gmail.com>
Date: Mon, 12 Nov 2018 02:42:25 +0800
Subject: [PATCH 4/4] gnu: teeworlds: Update to 0.7.0 [fixes CVE-2018-18541].

* gnu/packages/games.scm (teeworlds): Update to 0.7.0.
[source]: Remove all bundled libraries.
[arguments]: Adjust accordingly.
[inputs]: Use sdl2 instead of sdl and python-wrapper instead of python-2.
Add json-parser, libmd and pnglite.
* gnu/packages/patches/teeworlds-use-latest-wavpack.patch: Update it.
---
 gnu/packages/games.scm                        | 107 ++++++++++++------
 .../teeworlds-use-latest-wavpack.patch        |  72 +++++++++---
 2 files changed, 129 insertions(+), 50 deletions(-)

diff --git a/gnu/packages/games.scm b/gnu/packages/games.scm
index 3679aa09c..8817e4db8 100644
--- a/gnu/packages/games.scm
+++ b/gnu/packages/games.scm
@@ -35,6 +35,7 @@ 
 ;;; Copyright © 2018 Tim Gesthuizen <tim.gesthuizen@yahoo.de>
 ;;; Copyright © 2018 Madalin Ionel-Patrascu <madalinionel.patrascu@mdc-berlin.de>
 ;;; Copyright © 2018 Benjamin Slade <slade@jnanam.net>
+;;; Copyright © 2018 Alex Vong <alexvong1995@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -4139,31 +4140,41 @@  small robot living in the nano world, repair its maker.")
 (define-public teeworlds
   (package
     (name "teeworlds")
-    (version "0.6.4")
+    (version "0.7.0")
     (source (origin
               (method url-fetch)
-              (uri (string-append "https://github.com/teeworlds/teeworlds/"
-                                  "archive/" version "-release.tar.gz"))
+              (uri (string-append "https://github.com/teeworlds/teeworlds"
+                                  "/archive/" version ".tar.gz"))
               (file-name (string-append name "-" version ".tar.gz"))
               (sha256
                (base32
-                "1mqhp6xjl75l49050cid36wxyjn1qr0vjx1c709dfg1lkvmgs6l3"))
+                "1ih79qcfc44biiwyhc51gwvkyab4cy5hya9yc2bq8phf899fpz2q"))
               (modules '((guix build utils)))
-              (snippet
-               '(begin
-                  (for-each delete-file-recursively
-                            '("src/engine/external/wavpack/"
-                              "src/engine/external/zlib/"))
-                  #t))
+              (snippet ; remove bundled libraries
+               '(begin (delete-file-recursively "src/engine/external/")
+                       #t))
               (patches
                (search-patches "teeworlds-use-latest-wavpack.patch"))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? #f ; no tests included
+       #:modules ((guix build gnu-build-system)
+                  (guix build utils)
+                  (srfi srfi-26))
        #:phases
        (modify-phases %standard-phases
          (replace 'configure
            (lambda* (#:key outputs #:allow-other-keys)
+             (define (use-latest-json-parser file)
+               (substitute* file
+                 (("engine/external/json-parser/json\\.h")
+                  "json-parser/json.h")
+                 (("json_parse_ex\\(&JsonSettings, pFileData, aError\\);")
+                  "json_parse_ex(&JsonSettings,
+                                 pFileData,
+                                 strlen(pFileData),
+                                 aError);")))
+
              ;; Embed path to assets.
              (substitute* "src/engine/shared/storage.cpp"
                (("#define DATA_DIR.*")
@@ -4173,50 +4184,76 @@  small robot living in the nano world, repair its maker.")
                                "\"")))
 
              ;; Bam expects all files to have a recent time stamp.
-             (for-each (lambda (file)
-                         (utime file 1 1))
+             (for-each (cut utime <> 1 1)
                        (find-files "."))
 
              ;; Do not use bundled libraries.
              (substitute* "bam.lua"
-               (("if config.zlib.value == 1 then")
-                "if true then")
-               (("wavpack = .*")
-                "wavpack = {}
-settings.link.libs:Add(\"wavpack\")\n"))
+               (("local json = Compile.+$")
+                "local json = nil
+settings.link.libs:Add(\"jsonparser\")")
+               (("local md5 = Compile.+$")
+                "local md5 = nil
+settings.link.libs:Add(\"md\")")
+               (("local png = Compile.+$")
+                "local png = nil
+settings.link.libs:Add(\"pnglite\")")
+               (("local wavpack = Compile.+$")
+                "local wavpack = nil
+settings.link.libs:Add(\"wavpack\")")
+               (("if config\\.zlib\\.value == 1")
+                "settings.cc.flags:Add(\"-DLIBMD_MD5_ALADDIN\")
+if config.zlib.value"))
+             (substitute* "src/engine/shared/network_token.cpp"
+               (("engine/external/md5/md5\\.h")
+                "md5.h"))
+             (substitute* "src/engine/client/graphics_threaded.cpp"
+               (("engine/external/pnglite/pnglite\\.h")
+                "pnglite.h"))
              (substitute* "src/engine/client/sound.cpp"
-               (("#include <engine/external/wavpack/wavpack.h>")
-                "#include <wavpack/wavpack.h>"))
+               (("engine/external/wavpack/wavpack\\.h")
+                "wavpack/wavpack.h"))
+             (for-each use-latest-json-parser
+                       '("src/game/client/components/countryflags.cpp"
+                         "src/game/client/components/menus_settings.cpp"
+                         "src/game/client/components/skins.cpp"
+                         "src/game/client/localization.cpp"
+                         "src/game/editor/auto_map.h"
+                         "src/game/editor/editor.cpp"))
              #t))
          (replace 'build
            (lambda _
-             (zero? (system* "bam" "-a" "-v" "release"))))
+             (invoke "bam" "-a" "-v" "conf=release")))
          (replace 'install
            (lambda* (#:key outputs #:allow-other-keys)
-             (let* ((out  (assoc-ref outputs "out"))
-                    (bin  (string-append out "/bin"))
-                    (data (string-append out "/share/teeworlds/data")))
-               (mkdir-p bin)
-               (mkdir-p data)
-               (for-each (lambda (file)
-                           (install-file file bin))
-                         '("teeworlds" "teeworlds_srv"))
-               (copy-recursively "data" data)
+             (let* ((arch ,(system->linux-architecture
+                            (or (%current-target-system)
+                                (%current-system))))
+                    (build (string-append "build/" arch "/release/"))
+                    (data-built (string-append build "data/"))
+                    (out (assoc-ref outputs "out"))
+                    (bin (string-append out "/bin/"))
+                    (data (string-append out "/share/teeworlds/data/")))
+               (for-each (cut install-file <> bin)
+                         (map (cut string-append build <>)
+                              '("teeworlds" "teeworlds_srv")))
+               (copy-recursively data-built data)
                #t))))))
-    ;; FIXME: teeworlds bundles the sources of "pnglite", a two-file PNG
-    ;; library without a build system.
     (inputs
      `(("freetype" ,freetype)
        ("glu" ,glu)
+       ("json-parser" ,json-parser)
+       ("libmd" ,libmd)
        ("mesa" ,mesa)
-       ("sdl-union" ,(sdl-union (list sdl
-                                      sdl-mixer
-                                      sdl-image)))
+       ("pnglite" ,pnglite)
+       ("sdl2" ,sdl2)
+       ("sdl2-image" ,sdl2-image)
+       ("sdl2-mixer" ,sdl2-mixer)
        ("wavpack" ,wavpack)
        ("zlib" ,zlib)))
     (native-inputs
      `(("bam" ,bam)
-       ("python" ,python-2)
+       ("python" ,python-wrapper)
        ("pkg-config" ,pkg-config)))
     (home-page "https://www.teeworlds.com")
     (synopsis "2D retro multiplayer shooter game")
diff --git a/gnu/packages/patches/teeworlds-use-latest-wavpack.patch b/gnu/packages/patches/teeworlds-use-latest-wavpack.patch
index e9fd99108..3ad1340d2 100644
--- a/gnu/packages/patches/teeworlds-use-latest-wavpack.patch
+++ b/gnu/packages/patches/teeworlds-use-latest-wavpack.patch
@@ -1,10 +1,20 @@ 
-Downloaded from https://anonscm.debian.org/cgit/pkg-games/teeworlds.git/plain/debian/patches/new-wavpack.patch.
+Downloaded from https://salsa.debian.org/games-team/teeworlds/raw/master/debian/patches/new-wavpack.patch.
 
-This patch lets us build teeworlds with wavpack 5.1.0.
+From: Markus Koschany <apo@debian.org>
+Date: Thu, 25 Oct 2018 20:52:27 +0200
+Subject: new-wavpack
 
+Make wavpack compatible with Debian's version.
+---
+ src/engine/client/sound.cpp | 33 +++++++++++++++------------------
+ src/engine/client/sound.h   |  4 ----
+ 2 files changed, 15 insertions(+), 22 deletions(-)
+
+diff --git a/src/engine/client/sound.cpp b/src/engine/client/sound.cpp
+index 048ec24..80de3c5 100644
 --- a/src/engine/client/sound.cpp
 +++ b/src/engine/client/sound.cpp
-@@ -328,17 +328,14 @@ void CSound::RateConvert(int SampleID)
+@@ -325,10 +325,6 @@ void CSound::RateConvert(int SampleID)
  	pSample->m_NumFrames = NumFrames;
  }
  
@@ -12,10 +22,10 @@  This patch lets us build teeworlds with wavpack 5.1.0.
 -{
 -	return io_read(ms_File, pBuffer, Size);
 -}
--
- int CSound::LoadWV(const char *pFilename)
+ 
+ ISound::CSampleHandle CSound::LoadWV(const char *pFilename)
  {
- 	CSample *pSample;
+@@ -336,6 +332,8 @@ ISound::CSampleHandle CSound::LoadWV(const char *pFilename)
  	int SampleID = -1;
  	char aError[100];
  	WavpackContext *pContext;
@@ -24,17 +34,18 @@  This patch lets us build teeworlds with wavpack 5.1.0.
  
  	// don't waste memory on sound when we are stress testing
  	if(g_Config.m_DbgStress)
-@@ -351,19 +348,23 @@ int CSound::LoadWV(const char *pFilename
- 	if(!m_pStorage)
- 		return -1;
+@@ -349,25 +347,29 @@ ISound::CSampleHandle CSound::LoadWV(const char *pFilename)
+ 		return CSampleHandle();
  
+ 	lock_wait(m_SoundLock);
 -	ms_File = m_pStorage->OpenFile(pFilename, IOFLAG_READ, IStorage::TYPE_ALL);
 -	if(!ms_File)
 +	File = m_pStorage->OpenFile(pFilename, IOFLAG_READ, IStorage::TYPE_ALL, aWholePath, sizeof(aWholePath));
 +	if(!File)
  	{
  		dbg_msg("sound/wv", "failed to open file. filename='%s'", pFilename);
- 		return -1;
+ 		lock_unlock(m_SoundLock);
+ 		return CSampleHandle();
  	}
 +	else
 +	{
@@ -43,7 +54,14 @@  This patch lets us build teeworlds with wavpack 5.1.0.
  
  	SampleID = AllocID();
  	if(SampleID < 0)
- 		return -1;
+ 	{
+-		io_close(ms_File);
+-		ms_File = 0;
++		io_close(File);
++		File = 0;
+ 		lock_unlock(m_SoundLock);
+ 		return CSampleHandle();
+ 	}
  	pSample = &m_aSamples[SampleID];
  
 -	pContext = WavpackOpenFileInput(ReadData, aError);
@@ -51,7 +69,29 @@  This patch lets us build teeworlds with wavpack 5.1.0.
  	if (pContext)
  	{
  		int m_aSamples = WavpackGetNumSamples(pContext);
-@@ -419,9 +420,6 @@ int CSound::LoadWV(const char *pFilename
+@@ -385,8 +387,8 @@ ISound::CSampleHandle CSound::LoadWV(const char *pFilename)
+ 		if(pSample->m_Channels > 2)
+ 		{
+ 			dbg_msg("sound/wv", "file is not mono or stereo. filename='%s'", pFilename);
+-			io_close(ms_File);
+-			ms_File = 0;
++			io_close(File);
++			File = 0;
+ 			lock_unlock(m_SoundLock);
+ 			return CSampleHandle();
+ 		}
+@@ -401,8 +403,8 @@ ISound::CSampleHandle CSound::LoadWV(const char *pFilename)
+ 		if(BitsPerSample != 16)
+ 		{
+ 			dbg_msg("sound/wv", "bps is %d, not 16, filname='%s'", BitsPerSample, pFilename);
+-			io_close(ms_File);
+-			ms_File = 0;
++			io_close(File);
++			File = 0;
+ 			lock_unlock(m_SoundLock);
+ 			return CSampleHandle();
+ 		}
+@@ -429,9 +431,6 @@ ISound::CSampleHandle CSound::LoadWV(const char *pFilename)
  		dbg_msg("sound/wv", "failed to open %s: %s", pFilename, aError);
  	}
  
@@ -61,14 +101,16 @@  This patch lets us build teeworlds with wavpack 5.1.0.
  	if(g_Config.m_Debug)
  		dbg_msg("sound/wv", "loaded %s", pFilename);
  
-@@ -527,7 +525,5 @@ void CSound::StopAll()
- 	lock_unlock(m_SoundLock);
+@@ -560,7 +559,5 @@ bool CSound::IsPlaying(CSampleHandle SampleID)
+ 	return Ret;
  }
  
 -IOHANDLE CSound::ms_File = 0;
 -
  IEngineSound *CreateEngineSound() { return new CSound; }
  
+diff --git a/src/engine/client/sound.h b/src/engine/client/sound.h
+index ff357c0..cec2cde 100644
 --- a/src/engine/client/sound.h
 +++ b/src/engine/client/sound.h
 @@ -21,10 +21,6 @@ public:
@@ -81,4 +123,4 @@  This patch lets us build teeworlds with wavpack 5.1.0.
 -
  	virtual bool IsSoundEnabled() { return m_SoundEnabled != 0; }
  
- 	virtual int LoadWV(const char *pFilename);
+ 	virtual CSampleHandle LoadWV(const char *pFilename);
-- 
2.19.1