> It's better to add the patches at the end of the series file to keep the
> order of security updates (in case patches build on each other), this
> also groups all security updates after the release nicely and makes it
> easy to spot the last additions.
Thanks, an updated debdiff is in attachment.
I'll wait a bit to see if I can get a review from the QEMU team.
Cheers,
Hugo
--
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
@@ -127,3 +127,8 @@
security/CVE-2016-4020-i386-kvmvapic-initialise-imm32-variable.patch
security/CVE-2016-2857-net-check-packet-payload-length.patch
security/CVE-2015-5239-ui-vnc-limit-client_cut_text-msg-payload-si.patch
+
+# 3 patches to fix CVE-2016-7116, #836502
+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
Attachment:
signature.asc
Description: PGP signature