Subject: Using a +t /tmp for chpass(1)
To: None <tech-security@netbsd.org>
From: Elad Efrat <elad@NetBSD.org>
List: tech-security
Date: 10/09/2006 19:26:38
Hi,

Few days ago we discussed a change on #NetBSD-code that made it to our
chpass(1), and someone suggested I bring it up on a public list such
as this "just to make sure" -- so I am.

We used to write the temporary files, pw.XXXXX, to /etc, which created
a problem: a user who started chpass(1) could ^Z it, then SIGKILL it,
leaving a temporary file in /etc, slowly filling it up.

Solutions discussed varied from kernel changes to limit signals to
set-id processes (so that, for example, SIGKILL can't be sent; like
how we limit coredumps of set-id processes), but eventually we chose
a different solution.

What we did is this:

	static char tempname[] = "/tmp/pw.XXXXXX";

	[...]

	dfd = mkstemp(tempname);
	if (dfd < 0 || fcntl(dfd, F_SETFD, 1) < 0)
		(*Pw_error)(tempname, 1, 1);
	if (atexit(cleanup)) {
		cleanup();
		errx(1, "couldn't register cleanup");
	}
	if (stat(dirname(tempname), &sb) == -1)
		err(1, "couldn't stat `%s'", dirname(tempname));
	if (!(sb.st_mode & S_ISTXT))
		errx(1, "temporary directory `%s' is not sticky",
		    dirname(tempname));

	display(tempname, dfd, pw);
	edit(tempname, pw);

Where we first create the temporary file using mkstemp(), guaranteeing
we get a newly created temporary file with 0600 permissions, avoiding
TOCTOU races. We then check to see that our temp dir, /tmp (hard-coded)
is +t, so that nobody but us (and root) can modify the file.

Since it seems every operating system has its own way of doing this, I
ended up agreeing it's only reasonable to bring this up to see if any
of the folks who wasn't present during the Bugathon and/or didn't read
the log message can point out anything obviously flawed we did.

So, here goes -- is the above okay?

-e.

-- 
Elad Efrat