Create temporary directories using fsPromises.mkdtemp()

[BrianJDrake]

jco run creates a temporary directory using this complex, undocumented and possibly insecure (no verification that the directory is private or empty) code:

jco/src/cmd/run.js

Lines 11 to 13 in cbc396d

	function getTmpDir (name) {
	return resolve(tmpdir(), crypto.createHash('sha256').update(name).update(Math.random().toString()).digest('hex'));
	}

Node.js 10+ provides a function that does this task: fsPromises.mkdtemp(). According to nodejs/node#6142 (comment), that function is a thin binding to libc's mkdtemp, so it should be secure.

[guybedford]

@BrianJDrake can you please state what your exact concern is here? tmpdir() resolves the Node.js temporary directory. crypto.createHash returns a hex string. There's no vulnerability in resolving a hex string relative to the system tmpdir.

[BrianJDrake]

@guybedford You point out that getTmpDir() itself has no security vulnerability. Strictly speaking, that is true: all it does is compute a path.

But this (undocumented) function doesn't exist in a vacuum; it comes with unwritten expectations about how it should be used. It is reasonable to expect that getTmpDir() behaves similarly to mktemp(1) or mkdtemp(3), that is, it securely creates a directory accessible only to the current user. The key properties here are that the directory is new (and therefore does not contain any untrusted files) and accessible only to the current user. In fact, jco run depends on getTmpDir() behaving this way: it writes code to that directory, then executes it.

When getTmpDir() is expected to behave this way, but does not actually behave this way, then there may be a security vulnerability. There are several issues to consider here:

• The SHA256 hash function is considered secure today, but future discoveries may mean that its output is predictable.
• Math.random() is implementation-dependent, not cryptographically secure, and possibly predictable.
• Even if the directory name is not predictable, an attacker may be able to watch the temporary directory for any new subdirectories.
• getTmpDir() (and the code calling it) does not verify whether the directory exists or what permissions it has (and, if it tried to, it might create race conditions).

We can do away with all this complexity by just using a standard function like fsPromises.mkdtemp().

[guybedford]

If you'd like to create a PR to use fsPromises.mkdtemp() I'd have no problem with that personally.