Bug#756565: lives: Numerous insecure temporary files used in smogrify

[James Cowgill]

Hi,

[please don't change the subject to 'bug update' - it makes it harder to
follow threads and is totally pointless]

On 20/09/16 15:51, salsaman wrote:
> I would prefer to keep $tmpdir as it is, I dont see any reason to change
> it to $XDG_CACHE_HOME as this variable is only used internally to
> smogrify. Also using /lives would be confusing as there is already a
> .lives file and lives-dir which are both created in $HOME. Also the user
> can select the location of $tmpdir at install time so it may be outside
> $HOME.

I was referring to moving the ~/livestmp directory, not the $tmpdir
variable which is just an implementation detail. Incidentally since this
directory is world writable, it's probably vulnerable to the same
security bugs that /tmp is. Maybe what I said before is wrong and all
files in $tmpdir are also vulnerable?

> Regarding the other files:
> 
> /tmp/lives-symlinks is only used in the Dynebolic bootable disk.
> Effectively this can be ignored, since it is not used in normal operation.

Why does the code still exist then? Can you confirm that it can never
ever be executed?

I have not investigated much, but if it can be executed then it could
quite easily be used to allow editing of any file in a user's home
directory.

Attacker: ln -s $HOME_OTHER /tmp/lives-symlinks/$ALL_HANDLES

$linksdir = "/tmp/lives-symlinks/$handle/";
[some lines]
$com="/bin/mkdir -p \"$linksdir\"";
[above command exits without error]
smog_system("/bin/chmod -R 777 \"$linksdir\"");

[boom]

In this particular case the kernel default security settings will
probably stop you doing this, but that doesn't mean you can ignore the
issue. As the kernel reports an error when doing this, smogrify will
probably fail as well.

> /tmp/.smogrify.* is intended to be used when an external script calls
> "smogrify get_tempdir <key>". This may be employed by external scripts
> to get the working directory location for LiVES. The token <key> is
> appended to the end of the filename so this can be considered secure.
> The LiVES application itself never calls this.

Even using a random <key> doesn't make it secure. You have to ensure any
files in /tmp exist and are owned by the user running lives, before
attempting to read or write to them. Otherwise an attacker could put a
symlink there and cause the lives user to write to an arbitrary location.

For example, simply grepping the lives source reveals:
lives-plugins/plugins/playback/video/oggstream.c:
dummyvar=system("smogrify get_tempdir oggstream");

This allows any user to truncate any file owned by the lives user by
simply creating a symlink, and waiting for smogrify to be run.
 ln -s $IMPORTANT_FILE /tmp/.smogrify.oggstream


> Actually I have a TODO there: allow a (optional) second parameter to
> override the default directory location (/tmp); so I shall implement
> this so that scripts can provide an alternative to /tmp if they wish.
> 
> 
> /tmp/.smogval : this is not used, I believe you are again confused by
> $tmpdir which as noted points to the working directory (e.g.
> ~/livestmp). There IS a minor bug where "/tmp/.smogval*" is removed by a
> cleanup operation. This code should be removed as it is no longer relevant.

I just grepped for /tmp. That file might be ok in that case.

> The above changes are trivial and I will update here as soon as they are
> done.

Thanks,
James

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20160920/a77a4c52/attachment.sig>