Re: Issues with /tmp/mc-$USER directory



Hello!

> > 1) Check that /tmp/mc-$USER is ours.  I think if I do stat() and it says
> > that I'm the owner, no adversary will be able to replace the directory.
>
> lstat() instead of stat() will be okay. stat() can be bad if someone else
> owns a symlink which points to a file of yours, and in the next moment he
> removes/alters that symlink.  Again, portability issues... I'm afraid
> lstat() is not available everywhere :(

It's used already and nobody complained, so it's portable enough for our
purposes.

> This is okay, provided that you try many random filenames in step 2, not
> just one or two. In this case step 3 will only be reached under very rare
> circumstances (really hard spoofing by someone else or some setup problem
> with /tmp).

I have implemented a safer check for /tmp/mc-$USER.  If it fails, mc
checks if it can create temporary files in /tmp.  If it works, /tmp is
used.  If it doesn't work, tmpdir is set to /dev/null/, so the temporary
files cannot be created.  In any case when /tmp/mc-$USER cannot be
created, the user is informed.

The new code is attached.  If there are any problems with it, please let
me know.

I'm aware of the problem with predictable name in subshell.c (pipe for
tcsh), and I'll try to fix it in the meantime.

-- 
Regards,
Pavel Roskin
/*
 * Return the directory where mc should keep its temporary files.
 * This directory is (in Bourne shell terms) "${TMPDIR=/tmp}-$USER"
 * When called the first time, the directory is created if needed.
 * The first call should be done early, since we are using fprintf()
 * and not message() to report possible problems.
 */
const char *
mc_tmpdir (void)
{
    static char buffer[64];
    static const char *tmpdir;
    const char *sys_tmp;
    struct passwd *pwd;
    struct stat st;
    const char *error = NULL;

    /* Check if already initialized */
    if (tmpdir)
	return tmpdir;

    sys_tmp = getenv ("TMPDIR");
    if (!sys_tmp) {
	sys_tmp = TMPDIR_DEFAULT;
    }

    pwd = getpwuid (getuid ());
    g_snprintf (buffer, sizeof (buffer), "%s/mc-%s", sys_tmp,
		pwd->pw_name);
    canonicalize_pathname (buffer);

    if (lstat (buffer, &st) == 0) {
	/* Sanity check for existing directory */
	if (!S_ISDIR (st.st_mode))
	    error = _("%s is not a directory\n");
	else if (st.st_uid != getuid ())
	    error = _("Directory %s is not owned by you\n");
	else if (((st.st_mode & 0777) != 0700)
		 && (chmod (buffer, 0700) != 0))
	    error = _("Cannot set correct permissions for directory %s\n");
    } else {
	/* Need to create directory */
	if (mkdir (buffer, S_IRWXU) != 0) {
	    fprintf (stderr,
		     _("Cannot create temporary directory %s: %s\n"),
		     buffer, unix_error_string (errno));
	    error = "";
	}
    }

    if (!error) {
	tmpdir = buffer;
    } else {
	int test_fd;
	char *test_fn;
	int fallback_ok = 0;

	if (*error)
	    fprintf (stderr, error, buffer);

	/* Test if sys_tmp is suitable for temporary files */
	tmpdir = sys_tmp;
	test_fd = mc_mkstemps (&test_fn, "mctest", NULL);
	if (test_fd != -1) {
	    close (test_fd);
	    test_fd = open (test_fn, O_RDONLY);
	    if (test_fd != -1) {
		close (test_fd);
		unlink (test_fn);
		fallback_ok = 1;
	    }
	}

	if (fallback_ok) {
	    fprintf (stderr, _("Temporary files will be created in %s\n"),
		     sys_tmp);
	} else {
	    fprintf (stderr, _("Temporary files will not be created\n"));
	    tmpdir = "/dev/null/";
	}

	fprintf (stderr, "%s\n", _("Press any key to continue..."));
	getc (stdin);
    }

    return tmpdir;
}


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]