[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Wheezy update for qemu ?



Dear qemu maintainers,

I've prepared an LTS upload for qemu. It fixes CVE-2016-7116[0]. A debdiff is in
attachement. 

I've applied following patches, as suggested here[1]:
http://git.qemu.org/?p=qemu.git;a=commit;h=56f101ecce0eafd09e2daf1c4eeb1377d6959261
http://git.qemu.org/?p=qemu.git;a=commit;h=fff39a7ad09da07ef490de05c92c91f22f8002f2
http://git.qemu.org/?p=qemu.git;a=commit;h=805b5d98c649d26fc44d2d7755a97f18e62b438a

All patches applied well, with minor adaptations (line numbers changed).

It would be very helpful if you could review it, or even better, test it.

Thanks !

Regards,
 Hugo

[0] https://security-tracker.debian.org/tracker/CVE-2016-7116
[1] https://marc.info/?l=oss-security&m=147259351226835&w=2 

-- 
             Hugo Lefeuvre (hle)    |    www.owl.eu.com
4096/ ACB7 B67F 197F 9B32 1533 431C AC90 AC3E C524 065E
diff -Nru qemu-1.1.2+dfsg/debian/changelog qemu-1.1.2+dfsg/debian/changelog
--- qemu-1.1.2+dfsg/debian/changelog	2016-07-29 18:22:00.000000000 +0200
+++ qemu-1.1.2+dfsg/debian/changelog	2016-09-07 11:23:34.000000000 +0200
@@ -1,3 +1,11 @@
+qemu (1.1.2+dfsg-6+deb7u15) wheezy-security; urgency=high
+
+  * Non-maintainer upload by the LTS Team.
+  * CVE-2016-7116: 9p: directory traversal flaw in 9p virtio backend.
+    (Closes: #836502)
+
+ -- Hugo Lefeuvre <hle@debian.org>  Wed, 07 Sep 2016 11:23:34 +0200
+
 qemu (1.1.2+dfsg-6+deb7u14) wheezy-security; urgency=medium
 
   * Non-maintainer upload by the LTS team.
diff -Nru qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-fix-walk-root-dir.patch qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-fix-walk-root-dir.patch
--- qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-fix-walk-root-dir.patch	1970-01-01 01:00:00.000000000 +0100
+++ qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-fix-walk-root-dir.patch	2016-09-07 11:23:34.000000000 +0200
@@ -0,0 +1,108 @@
+Description: 9pfs: handle walk of ".." in the root directory
+ The 9P spec at http://man.cat-v.org/plan_9/5/intro says:
+ .
+ All directories must support walks to the directory .. (dot-dot) meaning
+ parent directory, although by convention directories contain no explicit
+ entry for .. or . (dot).  The parent of the root directory of a server's
+ tree is itself.
+ .
+ This means that a client cannot walk further than the root directory
+ exported by the server. In other words, if the client wants to walk
+ "/.." or "/foo/../..", the server should answer like the request was
+ to walk "/".
+ .
+ This patch just does that:
+ - we cache the QID of the root directory at attach time
+ - during the walk we compare the QID of each path component with the root
+   QID to detect if we're in a "/.." situation
+ - if so, we skip the current component and go to the next one
+Origin: upstream, http://git.qemu.org/?p=qemu.git;a=commit;h=56f101ecce0eafd09e2daf1c4eeb1377d6959261
+Bug-Debian: https://bugs.debian.org/836502
+--- a/hw/9pfs/virtio-9p.c	2012-09-07 19:30:45.000000000 +0200
++++ b/hw/9pfs/virtio-9p.c	2016-09-07 11:36:24.381894921 +0200
+@@ -981,6 +981,7 @@
+         goto out;
+     }
+     err += offset;
++    memcpy(&s->root_qid, &qid, sizeof(qid));
+     trace_v9fs_attach_return(pdu->tag, pdu->id,
+                              qid.type, qid.version, qid.path);
+     s->root_fid = fid;
+@@ -1215,6 +1216,14 @@
+     return offset;
+ }
+ 
++static bool not_same_qid(const V9fsQID *qid1, const V9fsQID *qid2)
++{
++    return
++        qid1->type != qid2->type ||
++        qid1->version != qid2->version ||
++        qid1->path != qid2->path;
++}
++
+ static void v9fs_walk(void *opaque)
+ {
+     int name_idx;
+@@ -1230,6 +1239,7 @@
+     V9fsFidState *newfidp = NULL;
+     V9fsPDU *pdu = opaque;
+     V9fsState *s = pdu->s;
++    V9fsQID qid;
+ 
+     err = pdu_unmarshal(pdu, offset, "ddw", &fid, &newfid, &nwnames);
+     if (err < 0) {
+@@ -1259,6 +1269,12 @@
+         err = -ENOENT;
+         goto out_nofid;
+     }
++
++    err = fid_to_qid(pdu, fidp, &qid);
++    if (err < 0) {
++        goto out;
++    }
++
+     v9fs_path_init(&dpath);
+     v9fs_path_init(&path);
+     /*
+@@ -1268,16 +1284,22 @@
+     v9fs_path_copy(&dpath, &fidp->path);
+     v9fs_path_copy(&path, &fidp->path);
+     for (name_idx = 0; name_idx < nwnames; name_idx++) {
+-        err = v9fs_co_name_to_path(pdu, &dpath, wnames[name_idx].data, &path);
+-        if (err < 0) {
+-            goto out;
+-        }
+-        err = v9fs_co_lstat(pdu, &path, &stbuf);
+-        if (err < 0) {
+-            goto out;
++        if (not_same_qid(&pdu->s->root_qid, &qid) ||
++            strcmp("..", wnames[name_idx].data)) {
++            err = v9fs_co_name_to_path(pdu, &dpath, wnames[name_idx].data,
++                                       &path);
++            if (err < 0) {
++                goto out;
++            }
++
++            err = v9fs_co_lstat(pdu, &path, &stbuf);
++            if (err < 0) {
++                goto out;
++            }
++            stat_to_qid(&stbuf, &qid);
++            v9fs_path_copy(&dpath, &path);
+         }
+-        stat_to_qid(&stbuf, &qids[name_idx]);
+-        v9fs_path_copy(&dpath, &path);
++        memcpy(&qids[name_idx], &qid, sizeof(qid));
+     }
+     if (fid == newfid) {
+         BUG_ON(fidp->fid_type != P9_FID_NONE);
+--- a/hw/9pfs/virtio-9p.h	2012-09-07 19:30:45.000000000 +0200
++++ b/hw/9pfs/virtio-9p.h	2016-09-07 11:38:27.817545667 +0200
+@@ -225,6 +225,7 @@
+     CoRwlock rename_lock;
+     int32_t root_fid;
+     Error *migration_blocker;
++    V9fsQID root_qid;
+ } V9fsState;
+ 
+ typedef struct V9fsStatState {
diff -Nru qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-file-names.patch qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-file-names.patch
--- qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-file-names.patch	1970-01-01 01:00:00.000000000 +0100
+++ qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-file-names.patch	2016-09-07 11:23:34.000000000 +0200
@@ -0,0 +1,146 @@
+Description: 9pfs: forbid . and .. in file names 
+ According to the 9P spec http://man.cat-v.org/plan_9/5/open about the
+ create request: The names . and .. are special; it is illegal to create
+ files with these names.
+ .
+ This patch causes the create and lcreate requests to fail with EINVAL if
+ the file name is either "." or "..". Even if it isn't explicitly written
+ in the spec, this patch extends the checking to all requests that may cause
+ a directory entry to be created:
+    - mknod
+    - rename
+    - renameat
+    - mkdir
+    - link
+    - symlink
+ .
+ The unlinkat request also gets patched for consistency (even if rmdir("foo/..")
+ is expected to fail according to POSIX.1-2001).
+ .
+ The various error values come from the linux manual pages.
+Origin: upstream, http://git.qemu.org/?p=qemu.git;a=commit;h=805b5d98c649d26fc44d2d7755a97f18e62b438a
+Bug-Debian: https://bugs.debian.org/836502
+--- a/hw/9pfs/virtio-9p.c	2016-09-07 21:42:33.785061440 +0200
++++ b/hw/9pfs/virtio-9p.c	2016-09-07 22:03:05.563638884 +0200
+@@ -1479,6 +1479,11 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
++        err = -EEXIST;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, dfid);
+     if (fidp == NULL) {
+         err = -ENOENT;
+@@ -2072,6 +2077,11 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
++        err = -EEXIST;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -EINVAL;
+@@ -2242,6 +2252,11 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
++        err = -EEXIST;
++        goto out_nofid;
++    }
++
+     dfidp = get_fid(pdu, dfid);
+     if (dfidp == NULL) {
+         err = -EINVAL;
+@@ -2323,6 +2338,11 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
++        err = -EEXIST;
++        goto out_nofid;
++    }
++
+     dfidp = get_fid(pdu, dfid);
+     if (dfidp == NULL) {
+         err = -ENOENT;
+@@ -2411,6 +2431,16 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data)) {
++        err = -EINVAL;
++        goto out_nofid;
++    }
++
++    if (!strcmp("..", name.data)) {
++        err = -ENOTEMPTY;
++        goto out_nofid;
++    }
++
+     dfidp = get_fid(pdu, dfid);
+     if (dfidp == NULL) {
+         err = -EINVAL;
+@@ -2523,6 +2553,11 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
++        err = -EISDIR;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -ENOENT;
+@@ -2640,6 +2675,12 @@
+         goto out_err;
+     }
+ 
++    if (!strcmp(".", old_name.data) || !strcmp("..", old_name.data) ||
++        !strcmp(".", new_name.data) || !strcmp("..", new_name.data)) {
++        err = -EISDIR;
++        goto out_err;
++    }
++
+     v9fs_path_write_lock(s);
+     err = v9fs_complete_renameat(pdu, olddirfid,
+                                  &old_name, newdirfid, &new_name);
+@@ -2808,6 +2849,7 @@
+     if (retval < 0) {
+         goto out_nofid;
+     }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         retval = -ENOENT;
+@@ -2858,6 +2900,11 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
++        err = -EEXIST;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -ENOENT;
+@@ -3016,6 +3063,11 @@
+         goto out_nofid;
+     }
+ 
++    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
++        err = -EEXIST;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -ENOENT;
diff -Nru qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-path-names.patch qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-path-names.patch
--- qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-path-names.patch	1970-01-01 01:00:00.000000000 +0100
+++ qemu-1.1.2+dfsg/debian/patches/security/CVE-2016-7116-forbid-illegal-path-names.patch	2016-09-07 11:23:34.000000000 +0200
@@ -0,0 +1,161 @@
+Description: 9pfs: forbid illegal path names 
+ Empty path components don't make sense for most commands and may cause
+ undefined behavior, depending on the backend.
+ .
+ Also, the walk request described in the 9P spec [1] clearly shows that
+ the client is supposed to send individual path components: the official
+ linux client never sends portions of path containing the / character for
+ example.
+ .
+ Moreover, the 9P spec [2] also states that a system can decide to restrict
+ the set of supported characters used in path components, with an explicit
+ mention "to remove slashes from name components".
+ .
+ This patch introduces a new name_is_illegal() helper that checks the
+ names sent by the client are not empty and don't contain unwanted chars.
+ Since 9pfs is only supported on linux hosts, only the / character is
+ checked at the moment. When support for other hosts (AKA. win32) is added,
+ other chars may need to be blacklisted as well.
+ .
+ If a client sends an illegal path component, the request will fail and
+ ENOENT is returned to the client.
+ .
+ [1] http://man.cat-v.org/plan_9/5/walk
+ [2] http://man.cat-v.org/plan_9/5/intro
+Origin: upstream, http://git.qemu.org/?p=qemu.git;a=commit;h=fff39a7ad09da07ef490de05c92c91f22f8002f2
+Bug-Debian: https://bugs.debian.org/836502
+--- a/hw/9pfs/virtio-9p.c	2016-09-07 13:56:08.048898484 +0200
++++ b/hw/9pfs/virtio-9p.c	2016-09-07 13:57:31.904294357 +0200
+@@ -1195,6 +1195,11 @@
+     complete_pdu(s, pdu, err);
+ }
+ 
++static bool name_is_illegal(const char *name)
++{
++    return !*name || strchr(name, '/') != NULL;
++}
++
+ static int v9fs_walk_marshal(V9fsPDU *pdu, uint16_t nwnames, V9fsQID *qids)
+ {
+     int i;
+@@ -1258,6 +1263,10 @@
+             if (err < 0) {
+                 goto out_nofid;
+             }
++            if (name_is_illegal(wnames[i].data)) {
++                err = -ENOENT;
++                goto out_nofid;
++            }
+             offset += err;
+         }
+     } else if (nwnames > P9_MAXWELEM) {
+@@ -1465,6 +1474,11 @@
+     }
+     trace_v9fs_lcreate(pdu->tag, pdu->id, dfid, flags, mode, gid);
+ 
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, dfid);
+     if (fidp == NULL) {
+         err = -ENOENT;
+@@ -2053,6 +2067,11 @@
+     }
+     trace_v9fs_create(pdu->tag, pdu->id, fid, name.data, perm, mode);
+ 
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -EINVAL;
+@@ -2218,6 +2237,11 @@
+     }
+     trace_v9fs_symlink(pdu->tag, pdu->id, dfid, name.data, symname.data, gid);
+ 
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     dfidp = get_fid(pdu, dfid);
+     if (dfidp == NULL) {
+         err = -EINVAL;
+@@ -2294,6 +2318,11 @@
+     }
+     trace_v9fs_link(pdu->tag, pdu->id, dfid, oldfid, name.data);
+ 
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     dfidp = get_fid(pdu, dfid);
+     if (dfidp == NULL) {
+         err = -ENOENT;
+@@ -2376,6 +2405,12 @@
+     if (err < 0) {
+         goto out_nofid;
+     }
++
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     dfidp = get_fid(pdu, dfid);
+     if (dfidp == NULL) {
+         err = -EINVAL;
+@@ -2482,6 +2517,12 @@
+     if (err < 0) {
+         goto out_nofid;
+     }
++
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -ENOENT;
+@@ -2594,6 +2635,11 @@
+         goto out_err;
+     }
+ 
++    if (name_is_illegal(old_name.data) || name_is_illegal(new_name.data)) {
++        err = -ENOENT;
++        goto out_err;
++    }
++
+     v9fs_path_write_lock(s);
+     err = v9fs_complete_renameat(pdu, olddirfid,
+                                  &old_name, newdirfid, &new_name);
+@@ -2807,6 +2853,11 @@
+     }
+     trace_v9fs_mknod(pdu->tag, pdu->id, fid, mode, major, minor);
+ 
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -ENOENT;
+@@ -2960,6 +3011,11 @@
+     }
+     trace_v9fs_mkdir(pdu->tag, pdu->id, fid, name.data, mode, gid);
+ 
++    if (name_is_illegal(name.data)) {
++        err = -ENOENT;
++        goto out_nofid;
++    }
++
+     fidp = get_fid(pdu, fid);
+     if (fidp == NULL) {
+         err = -ENOENT;
diff -Nru qemu-1.1.2+dfsg/debian/patches/series qemu-1.1.2+dfsg/debian/patches/series
--- qemu-1.1.2+dfsg/debian/patches/series	2016-07-29 18:22:00.000000000 +0200
+++ qemu-1.1.2+dfsg/debian/patches/series	2016-09-07 11:23:34.000000000 +0200
@@ -1,3 +1,8 @@
+# 3 patches to fix CVE-2016-7116
+security/CVE-2016-7116-fix-walk-root-dir.patch
+security/CVE-2016-7116-forbid-illegal-path-names.patch
+security/CVE-2016-7116-forbid-illegal-file-names.patch
+
 02_kfreebsd.patch
 qemu-ifunc-sparc.patch
 configure-nss-usbredir.patch

Attachment: signature.asc
Description: PGP signature


Reply to: