Re: [PATCH 04/11] uml - hostfs: avoid possible escapes from hostfs=.

[Jeff Dike]

On Mon, Mar 05, 2007 at 09:49:02PM +0100, Paolo 'Blaisorblade' Giarrusso wrote:
> From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>
> 
> Avoid accepting things like -o .., -o dir/../../dir2, -o dir/../.. .
> This may be considered useless, but YMMV. I consider that this has a limited
> security value, exactly like disabling module support (in many case it is
> useful).

Two comments on this one:

> +	return  strstr(path, "/../") != NULL ||
> +		strcmp(path, "..") == 0 ||
> +		strncmp(path, "../", strlen("../")) == 0 ||
> +		str_ends_with(path, "/..");

Minor style point - I'd be happier with more parens:

+	return  (strstr(path, "/../") != NULL) ||
+		(strcmp(path, "..") == 0) ||
+		(strncmp(path, "../", strlen("../")) == 0) ||
+		str_ends_with(path, "/..");

C gets operator precedence wrong in one or two cases, so I just put parens
any place it might matter.

Second, there is code in externfs which does the same thing without
parsing paths which you might consider instead.  It sees whether the
requested directory is outside the jail by cd-ing to it and then
repeatedly cd .. until it either reaches / or the jail root.

A copy is below for your reading pleasure.

				Jeff

-- 
Work email - jdike at linux dot intel dot com


/* This does a careful check of whether it's being asked to mount something
 * that's outside the hostfs jail.  It does so by cd-ing to the requested
 * directory and "cd .." until it either reaches / or the jail directory.
 * If it reaches /, then we were outside the jail and we return -EPERM.
 * I do things this way rather than trying to examine the passed-in root
 * to avoid having to parse the string for instances of ".." and the like.
 * Examining the string also doesn't help with symlinks that point outside
 * the jail.  Plus, if there's no parsing, there's no parser to try to exploit.
 */
static int escaping_jail(char *root)
{
	struct uml_stat jail_inode;
	struct uml_stat last_dir, cur_dir;
	int save_pwd, err, escaping = -EPERM;
	const char *path[] = { jail_dir, root, NULL };
	char tmp[HOSTFS_BUFSIZE], *file;

	err = os_stat_file(jail_dir, &jail_inode);
	if(err)
		goto out;

	save_pwd = os_open_file(".", of_read(OPENFLAGS()), 0);
	if(save_pwd < 0)
		goto out;

	file = get_path(path, tmp, sizeof(tmp));
	if(file == NULL)
		goto out_close;

	err = os_change_dir(file);
	if(err)
		goto out_free;

	err = os_stat_file(".", &cur_dir);
	if(err)
		goto out_back;

	do {
		if(SAME_INO(cur_dir, jail_inode)){
			escaping = 0;
			break;
		}

		err = os_change_dir("..");
		if(err)
			goto out_back;

		last_dir = cur_dir;

		err = os_stat_file(".", &cur_dir);
		if(err)
			goto out_back;

	} while(!SAME_INO(last_dir, cur_dir));

	err = os_fchange_dir(save_pwd);
	if(err)
		goto out_close;

	free_path(file, tmp);
	os_close_file(save_pwd);

	return escaping;

out_back:
	os_fchange_dir(save_pwd);
out_free:
	free_path(file, tmp);
out_close:
	os_close_file(save_pwd);
out:
	return err;
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/