Bug#756600: xcfa: Insecure use of temporary files, subject to race conditions

[Steve Kemp]

Package: xcfa
Version: 4.3.1-1
Severity: important
Tags: security

xcfa contains several insecure uses of temporary files.

For example the file src/get_info.c has code to test that
curl is present, in the function GetInfo_wget which
essentially runs:

	wget --user-agent=\"Mozilla 22.0\" --directory-prefix=/tmp/  http://google.fr/
        ..
        if [ -e /tmp/index.html ]; then 
		rm /tmp/index.html
	fi

This is probably safe, because wget will not follow symlinks, and will
instead create "index.html.1" - but any existing file called /tmp/index.html
will be removed regardless.

More serious issues exist throughout the codebase.  For example the
code in dvdread_create_recap_audio, located in src/dvd_read.c contains
this lovely function:

        // Suppression du fichier precedant si il existe
        g_unlink ("/tmp/get_infos_dvd.sh");
        g_unlink ("/tmp/infos_dvd.txt");

        fp = fopen ("/tmp/get_infos_dvd.sh", "w");

        fprintf (fp, "#!/bin/sh\n");
        fprintf (fp, "\n");
        fprintf (fp, "set -e\n");
        fprintf (fp, "\n");

	..
	..

        system ("chmod +x /tmp/get_infos_dvd.sh");

        system ("/tmp/get_infos_dvd.sh");
        g_unlink ("/tmp/get_infos_dvd.sh");


Similarly the code which copies files to the trashbin, located in src/file_trash.c,
has some nice code which runs:

        system ("env | grep \"KDE_FULL_SESSION\" > /tmp/tst_kde_full_session.txt");
        if ((fp = fopen ("/tmp/tst_kde_full_session.txt", "r")) != NULL) {
                while (fgets (buf, MAX_CARS_KDE, fp) != NULL) {
                        if (strcmp (buf, "KDE_FULL_SESSION") == 0) {
                                if (strcmp (buf, "true") == 0 || strcmp (buf, "TRUE") == 0) {
                                        BoolRet = TRUE;
                                        break;
                                }
                        }
                }
                fclose (fp);
        }
        g_unlink ("/tmp/tst_kde_full_session.txt");


In short this codebase is rife with race-conditions allowing arbitrary shell executation,
via /tmp/get_infos_dvd.sh, and file truncation/deletion.

I'd strongly urge the maintainer to audit the codebase for additional issues, with the
help of upstream.



Steve
--



-- System Information:
Debian Release: 7.6
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.14-0.bpo.1-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_US.UTF8, LC_CTYPE=en_US.UTF8 (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF8)
Shell: /bin/sh linked to /bin/dash