Bug#308783: libxpm4: problems with s_popen (CAN-2004-0914)

[Matej Vela <vela@debian.org>]

Package: libxpm4
Version: 4.3.0.dfsg.1-12
Severity: grave
Justification: may allow access to the accounts of users who use the package

The CAN-2004-0914 patch introduced a s_popen() function as a safe
replacement for popen().  Instead of invoking a shell, it splits
arguments on whitespace and passes the command directly to execvp(3).
However, it doesn't handle quoting or redirection, so code like

  WrFFrI.c:339:       snprintf(buf, sizeof(buf), "gzip -q > \"%s\"", filename);
  WrFFrI.c:340:       if (!(mdata->stream.file = s_popen(buf, "w")))

results in a ">" argument and superfluous quotes:

  execve("/bin/gzip", ["gzip", ">", "\"foo.gz\""], [/* 19 vars */])

This completely breaks the transparent compression and decompression.

Furthermore, since gzip processes all arguments regardless of errors, an
attacker can use filenames with whitespace to compress arbitrary files:
(xpmtest taken from <https://bugs.freedesktop.org/show_bug.cgi?id=1920>)

  # ./xpmtest crab.xpm 'fnord -v /etc/hosts.deny fnord.gz'
  w=28, h=28, cpp=2, cols=6, vmask=00000000, hotspot=0,0
  gzip: >: No such file or directory
  gzip: "fnord: No such file or directory
  /etc/hosts.deny:       -50.0% -- replaced with /etc/hosts.deny.gz
  gzip: fnord.gz": No such file or directory

The above would effectively disable TCP wrappers.  The -r option can be
used to compress whole directory trees.

s_popen() also has issues with error handling, signals, and runaway
child processes.  All of this has been fixed in X11R6.8.2, though I
don't think they're aware of the security implications (patches at
<http://ftp.x.org/pub/X11R6.8.1/patches/> are still vulnerable).

Thanks,

Matej