Commit a0e640a8 authored by Greg Kurz's avatar Greg Kurz
Browse files

9pfs: local: remove: don't follow symlinks 



The local_remove() callback is vulnerable to symlink attacks because it
calls:

(1) lstat() which follows symbolic links in all path elements but the
    rightmost one
(2) remove() which follows symbolic links in all path elements but the
    rightmost one

This patch converts local_remove() to rely on opendir_nofollow(),
fstatat(AT_SYMLINK_NOFOLLOW) to fix (1) and unlinkat() to fix (2).

This partly fixes CVE-2016-9602.

Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
parent df4938a6

hw/9pfs/9p-local.c

+21 −43
Original line number Diff line number Diff line
@@ -1021,54 +1021,32 @@ err_out: 


static int local_remove(FsContext *ctx, const char *path)

{

    int err;

    struct stat stbuf;

    char *buffer;

    char *dirpath = g_path_get_dirname(path);

    char *name = g_path_get_basename(path);

    int flags = 0;

    int dirfd;

    int err = -1;



    if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {

        buffer = rpath(ctx, path);

        err =  lstat(buffer, &stbuf);

        g_free(buffer);

        if (err) {

            goto err_out;

        }

        /*

         * If directory remove .virtfs_metadata contained in the

         * directory

         */

        if (S_ISDIR(stbuf.st_mode)) {

            buffer = g_strdup_printf("%s/%s/%s", ctx->fs_root,

                                     path, VIRTFS_META_DIR);

            err = remove(buffer);

            g_free(buffer);

            if (err < 0 && errno != ENOENT) {

                /*

                 * We didn't had the .virtfs_metadata file. May be file created

                 * in non-mapped mode ?. Ignore ENOENT.

                 */

                goto err_out;

            }

    dirfd = local_opendir_nofollow(ctx, dirpath);

    if (dirfd) {

        goto out;

    }

        /*

         * Now remove the name from parent directory

         * .virtfs_metadata directory

         */

        buffer = local_mapped_attr_path(ctx, path);

        err = remove(buffer);

        g_free(buffer);

        if (err < 0 && errno != ENOENT) {

            /*

             * We didn't had the .virtfs_metadata file. May be file created

             * in non-mapped mode ?. Ignore ENOENT.

             */



    if (fstatat(dirfd, path, &stbuf, AT_SYMLINK_NOFOLLOW) < 0) {

        goto err_out;

    }



    if (S_ISDIR(stbuf.st_mode)) {

        flags |= AT_REMOVEDIR;

    }



    buffer = rpath(ctx, path);

    err = remove(buffer);

    g_free(buffer);

    err = local_unlinkat_common(ctx, dirfd, name, flags);

err_out:

    close_preserve_errno(dirfd);

out:

    g_free(name);

    g_free(dirpath);

    return err;

}