xorl %eax, %eax

OpenSSH ssh-keysign Utility File Descriptor Leak

leave a comment »

As we can read in the official security advisory, this bug was reported in private by Tomas Mraz. The vulnerability affects OpenSSH prior to version 5.8p2 on platforms configured to use ‘ssh-rand-helper‘ for entropy.

Here is the buggy code from openssh/ssh-keysign.c file.

int
main(int argc, char **argv)
{
	Buffer b;
	Options options;
	Key *keys[2], *key = NULL;
	struct passwd *pw;
	int key_fd[2], i, found, version = 2, fd;
	u_char *signature, *data;
	char *host;
	u_int slen, dlen;
	u_int32_t rnd[256];

	key = NULL;	/* XXX gcc */

	/* Ensure that stdin and stdout are connected */
	if ((fd = open(_PATH_DEVNULL, O_RDWR)) < 2)
		exit(1);
	/* Leave /dev/null fd iff it is attached to stderr */
	if (fd > 2)
		close(fd);

	key_fd[0] = open(_PATH_HOST_RSA_KEY_FILE, O_RDONLY);
	key_fd[1] = open(_PATH_HOST_DSA_KEY_FILE, O_RDONLY);

	original_real_uid = getuid();	/* XXX readconf.c needs this */
	if ((pw = getpwuid(original_real_uid)) == NULL)
		fatal("getpwuid failed");
	pw = pwcopy(pw);

	permanently_set_uid(pw)
  ...
	if (key_fd[0] == -1 && key_fd[1] == -1)
		fatal("could not open any host key");
  ...
	if (ssh_msg_send(STDOUT_FILENO, version, &b) == -1)
		fatal("ssh_msg_send failed");

	return (0);
}

As you can see, it opens two file descriptors for the RSA and DSA key files as read-only. However, there is no check that the aforementioned file descriptors are closed when a new process is spawned.
Due to this missing check users could hijack those leaked file descriptors in order to read the ‘_PATH_HOST_RSA_KEY_FILE’ and ‘_PATH_HOST_DSA_KEY_FILE’ files containing the host keys and potentially use them to elevate their privileges.

The fix was to update the file descriptors’ flags to include the ‘FD_CLOEXEC’ flag in order to close the file descriptor(s) when a new process will be executed.

 	key_fd[1] = open(_PATH_HOST_DSA_KEY_FILE, O_RDONLY);
+	if (fcntl(key_fd[0], F_SETFD, FD_CLOEXEC) != 0 ||
+	    fcntl(key_fd[1], F_SETFD, FD_CLOEXEC) != 0)
+		fatal("fcntl failed");
 
 	original_real_uid = getuid();	/* XXX readconf.c needs this */

Written by xorl

May 9, 2011 at 20:47

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s