insecure temporary files

[kernigh]

ACK fails to remove some temporary files, uses weak permissions for temporary files, and may overwrite existing files with temporary files. I am looking for a better way to make temporary files.

My last clean build (of my pull request #121) failed to remove 32 temporary files:

$ cd /tmp
$ ls tmp.*
tmp.0.Gvf5IeNek tmp.0.oijcl9bQk tmp.1.qeOiR42nV tmp.2.kUHDqX6zZ tmp.3.OqRwFGICn
tmp.0.Gz5rUMNaQ tmp.1.8XGW3fUa8 tmp.1.z4x0anVvQ tmp.2.nMBnLfLKv tmp.3.c6J2xsx06
tmp.0.KvMC3AHtA tmp.1.KCR0yxO3s tmp.2.0HmG1zoP7 tmp.2.wnyRNS3Pa tmp.3.cbpGkgDnA
tmp.0.LR45bFmU4 tmp.1.LBhkiEsvL tmp.2.98N5DriUr tmp.3.4Qzskej2F tmp.3.pu3Xs741Q
tmp.0.SIE5FbbxB tmp.1.eB6ejJp5g tmp.2.ESnNvpZS5 tmp.3.4YifxQra1
tmp.0.UwJ44TU0X tmp.1.fAu9QuEIE tmp.2.O1xebovgj tmp.3.D7ZXserCM
tmp.0.mFc9HBo5k tmp.1.n6729qFBb tmp.2.Vo3Yft00q tmp.3.J6IK15Syn

I have accumulated several such files while rebuilding or using ACK, but I rebooted my system before this build, and the reboot cleaned /tmp. I don't know which ACK tool left these files; their names don't include the name of the tool. Names like tmp.%d.%9s come from tmpnam() in OpenBSD 6.6; the %d from 0 to 3 suggests that some tool creates 4 temporary files without cleaning them.

The files have weak permissions, because all users can read them:

$ ls -l /tmp/tmp.0.Gv*
-rw-r--r--  1 kernigh  wheel  29005 Nov  5 15:53 /tmp/tmp.0.Gvf5IeNek

The permissions -rw-r--r-- conform to my umask 022. The problem is that the file is in /tmp. I might run ACK in directories that other users can't reach, but when ACK writes files with weak permissions to /tmp, it leaks my data to other users.

A temporary file risks overwriting an existing file. For example, util/arch/archiver.c line 344 calls sys_tmpnam(temp_arch), and later line 414 calls open_archive(temp_arch, CREATE): where sys_tmpnam() does tmpnam(temp_arch) and open_archive() does fopen(temp_arch, "wb+"). Beware that tmpnam() names a file but does not create it, and fopen() truncates an existing file.

A good tmpnam() would check that the file doesn't exist, but there is a race between tmpnam() and fopen(), where someone else may create a file with the same name. (This is a TOCTOU race: time of check, time of use.) Another user might perform a symlink attack (like from /tmp/tmp.0.Gvf5IeNek to /home/kernigh/.profile); or processes might collide by using the same name for two different temporary files.

ACK doesn't always use tmpnam(). The code in util/ack/files.c seems to create several names /tmp/Ack_%x%s (where %x is getpid() and %s is a counter in digits a-z), then util/ack/run.c seems to create or truncate the named files.

The O_EXCL flag to open(2) would avoid overwriting an existing file, but O_EXCL did not exist in Unix v7, so ACK doesn't use O_EXCL.

[davidgiven]

tmpnam() is known to be flawed. The best option here is something like tmpfile() or mkstemp() which atomically creates and opens (and deletes) a file, so it's not accessible by other processes and is guaranteed not to conflict with anything.

I'd probably argue that we should convert the ACK tools to use tmpfile() or mkstemp() whereever possible, because of the problems you describe, and provide a basic insecure fallback implementation of mkstemp() in the ACK libc itself for platforms which can't support it. We should probably add O_EXCL as a standard symbol, too.

[kernigh]

I found the tool that fails to remove temporary files: it's em_ass, the linker for ack -mem22. The build links 8 examples/*.em22, and each link drops 4 files, so this explains all 32 files.

This is how some other compilers name their temporary files (in this example, a relocatable object to pass to the linker):

• clang: /tmp/example-683c77.o
• cparser: /tmp//4Dvv88/a-0-1.o
• gcc: /tmp//ccnFIXmy.o
• pcc: /tmp/ctm.b0wTKS

By tracing system calls, I observed clang, gcc, and pcc using open(2) O_EXCL mode 0600; and cparser using mkdir(2) mode 0700.

I like clang's /tmp/example-683c77.o because it has the .o suffix and it hints that example.c is the source. mkstemp() can't put the .o suffix, so I would use mkstemps() like mkstemps("/tmp/example-XXXXXX.o", 2). mkstemps() isn't a standard function, but I saw mkstemps() in many libc (multiple BSDs, illumos, Linux glibc, Linux musl). mkstemp() is in POSIX. tmpfile() is in standard C; ACK already uses tmpfile() in mach/proto/as/comm4.c.

[kernigh]

In my kernigh-tmp branch, I changed how some tools make temporary files:

• util/ass switched to tmpfile()
• util/LLgen switched to mkstemp()
• util/ego/il began to check if mkstemp() failed
• util/ego/em_ego switched to mkdtemp()
• util/ack switched to mkdtemp()

util/ass was failing to remove its temporary files; the switch to tmpfile() fixes it.

I had mentioned using mkstemps() to put suffixes on temporary files, but in this branch, I used mkdtemp() instead, so util/ack might preprocess main.c into /tmp/Ack_LePuwF/a.i. (I would prefer main.i over a.i, but didn't make that change.)

I have not finished this branch; there are about 5 or 6 more tools that make temporary files.