Eh? Calling system() for a binary without a path? And why system() using execl() in the first place, when you could do something using execve() without a sh inbetween instead?

Even w/o an exploit this can be prettier and more secure.

We're not disagreeing. Even if there's no 'sploit there, people have spaces in their directory or file names, and it's kinda nice for your tool to work with those, so obviously you should be using an execve variant to pass arguments properly.

I assume the reason for the incorrect system call is that doing a shell redirect ('>') does actually look prettier though.

Doing the actual right code is definitely less pretty looking IMO: https://github.com/luvit/zlib/blob/8de57bce969eb9dafc1f1f5c2...

Agree as a basic example. tmpname1 = "/tmp/file.txt; rm -rf /"; becomes gunzip -c /tmp/file.txt; rm -rf / > /tmp/atopwrkXXXXXX

Also tmpname2 could be symlinked to /etc/passwd before it is unlinked..

> Also tmpname2 could be symlinked to /etc/passwd before it is unlinked..

Yeah, sure, but only if you run atop as root, otherwise it'll just get a "permission denied", and if you can run atop as root with whatever flags you like, you might as well just run 'rm' instead.

It's not a suid binary, so while it's bad code and a smell, I don't think the TOCTOU is a security issue in how it's commonly run (i.e. as an interactive CLI running as your user).

The TOCTOU is relevant (without suid) if someone can quickly make the right prediction of the tmpname2 value that's generated by the PRNG used by mkstemp, and create a symlink with that value before gunzip is executed. After calling mkstemp, the code should use the returned file descriptor, and thereby eliminate all TOCTOU risk. However, on (perhaps?) most devices that would realistically use atop, the PRNG works well enough that that prediction would fail.