[bug#38687] gnu: Add libtcod.

On 2019-12-23 18:37, Ludovic Courtès wrote:
> Hi,
> 
> Giacomo Leidi <goodoldpaul@autistici.org> skribis:
> 
>> * gnu/packages/game-development.scm (libtcod): New variable.
> 
> Could you (1) add a ‘snippet’ that removes zlib and utf8proc from
> src/vendor (assuming it doesn’t break the build system…), and (2) add
> utf8proc as an input?
> 
> That way we’d make sure we don’t end up using bundled copies of these
> packages
> 
> Apart from that it LGTM, thanks!
> 
> Ludo’.

Hi Ludo',
Sorry for the delay. I checked and it seems that a lot of vendor library 
paths are hardcoded in the C headers.

I made a new patch with a snippet that deletes zlib sources, but I'm 
unsure about how to approach the hardcoded paths.

grepping for 'vendor' inside the source returns


src/libtcod/sys_sdl_img_png.cpp
41:#include "../vendor/lodepng.h"

src/libtcod/console/printing.cpp
54:#include "../../vendor/utf8proc/utf8proc.h"

src/libtcod/sdl2/gl2_ext_.h
36:#include "../../vendor/glad.h"

src/libtcod/color/canvas.cpp
39:#include "../../vendor/lodepng.h"

src/libtcod/tileset/truetype.cpp
43:#include "../../vendor/stb_truetype.h"

src/libtcod/tileset/tilesheet.cpp
34:#include "../../vendor/lodepng.h"


As far as I understood (I'm no C wizard) lodepng, glad and stb are 
libraries which are designed to be dropped inside a source tree and 
statically built in binaries. Right now I think those are secondary 
problems and the main deal is how to effectively replace the included 
utf8proc.

I'm not sure if replacing every "#include ../../vendor/something.h" with 
"#include <something.h>" ( by including a .patch file or with 
substitute* ? Food for thought :D) and then placing something.h inside a 
path known by the C linker is enough but it's the  I'm not really sure 
where to go from here.

Any advice is very welcome, thanks.

Giacomo

goodoldpaul@autistici.org writes:

> As far as I understood (I'm no C wizard) lodepng, glad and stb are 
> libraries which are designed to be dropped inside a source tree and 
> statically built in binaries. Right now I think those are secondary 
> problems and the main deal is how to effectively replace the included 
> utf8proc.
>
> I'm not sure if replacing every "#include ../../vendor/something.h" with 
> "#include <something.h>" ( by including a .patch file or with 
> substitute* ? Food for thought :D) and then placing something.h inside a 
> path known by the C linker is enough but it's the  I'm not really sure 
> where to go from here.

Something like

(substitute* (find-files "src/libtcod" "\\.(h|cpp)$")
  (("\\.\\./\\.\\./vendor/") ""))

Would make all the header paths relative, i.e. looked up in the compiler
search path.  In practice you might need to narrow down the substitution
so that e.g. vendor/utf8proc is preserved, if it turns out to be
difficult to unbundle.

There is currently no stb-truetype package, but it should be trivial to
add it (see gnu/packages/stb.scm).  Lodepng looks fairly easy too.  :-)

Good luck!

Hi Marius and Ludo,
I managed to remove all vendored libraries except for glad.h which seems 
to be some kind of generated glue code for loading OpenGL 
(https://github.com/Dav1dde/glad). In the next two patches I'm adding 
libtcod and it's dependency lodepng.

Guix lint is warning me that lodepng could be affected by 
CVE-2019-17178, but taking a look at 
https://nvd.nist.gov/vuln/detail/CVE-2019-17178 and 
https://nvd.nist.gov/vuln/search/results?adv_search=true&cpe_version=cpe%3a%2fa%3alodev%3alodepng%3a2019-09-28 
seems to indicate that lodepng should  be *not* vulnerable since 
28/09/2019, did I understand correctly?

Please don't hesitate and tell me if anything should done w.r.t. the 
CVE.

Thanks for your patience reviewing this,

Giacomo

goodoldpaul@autistici.org writes:

> Hi Marius and Ludo,
> I managed to remove all vendored libraries except for glad.h which seems 
> to be some kind of generated glue code for loading OpenGL 
> (https://github.com/Dav1dde/glad). In the next two patches I'm adding 
> libtcod and it's dependency lodepng.

Excellent, thanks for taking the time to get rid of the bundled
dependencies.

> Guix lint is warning me that lodepng could be affected by 
> CVE-2019-17178, but taking a look at 
> https://nvd.nist.gov/vuln/detail/CVE-2019-17178 and 
> https://nvd.nist.gov/vuln/search/results?adv_search=true&cpe_version=cpe%3a%2fa%3alodev%3alodepng%3a2019-09-28 
> seems to indicate that lodepng should  be *not* vulnerable since 
> 28/09/2019, did I understand correctly?
>
> Please don't hesitate and tell me if anything should done w.r.t. the 
> CVE.

The CVE entry points to this commit:

https://github.com/FreeRDP/FreeRDP/commit/9fee4ae076b1ec97b97efb79ece08d1dab4df29a

Which changes something in FreeRDP's bundled version of LodePNG.  The
changes in question do not seem to be in upstream LodePNG:

https://github.com/lvandeve/lodepng/blob/master/lodepng.cpp#L1079

It's not clear to me whether this is a problem with LodePNG, or just
improper use of its API.  It looks like the latter: tree->lengths is
checked just below the changed line, so FreeRDP must be catching the
83 return code and keep going to get the memory leak described in the
CVE entry.

We can either ignore it using the 'lint-hidden-cve' property, and add a
comment that this version of LodePNG should not be used with FreeRDP; or
take the patch from FreeRDP, as it looks innocent enough.  I don't
really have a strong opinion here, nor sufficient expertise, so I'd be
happy if others could chime in.