Your message dated Thu, 21 Jun 2012 13:08:52 -0700
with message-id <20120621200852.GS2630@talon.fglan>
and subject line Re: Bug#678280: CVE-2012-2652
has caused the Debian Bug report #678280,
regarding CVE-2012-2652
to be marked as done.
This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.
(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)
--
678280: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=678280
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
--- Begin Message ---
Package: qemu
Severity: grave
Tags: security
Please see https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2012-2652 for
details and a reference to the upstream patch.
Cheers,
Moritz
--- End Message ---
--- Begin Message ---
Version: 1.1.0+dfsg-1
On Wed, Jun 20, 2012 at 05:19:55PM +0200, Moritz Muehlenhoff wrote:
> Please see https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2012-2652 for
> details and a reference to the upstream patch.
This is fixed upstream, and included in qemu 1.1.0+dfsg-1, currently present in
sid and wheezy in the following upstream git commit:
commit eba25057b9a5e19d10ace2bc7716667a31297169
Author: Jim Meyering <j...@meyering.net>
Date: Mon May 28 09:27:54 2012 +0200
block: prevent snapshot mode $TMPDIR symlink attack
In snapshot mode, bdrv_open creates an empty temporary file without
checking for mkstemp or close failure, and ignoring the possibility
of a buffer overrun given a surprisingly long $TMPDIR.
Change the get_tmp_filename function to return int (not void),
so that it can inform its two callers of those failures.
Also avoid the risk of buffer overrun and do not ignore mkstemp
or close failure.
Update both callers (in block.c and vvfat.c) to propagate
temp-file-creation failure to their callers.
get_tmp_filename creates and closes an empty file, while its
callers later open that presumed-existing file with O_CREAT.
The problem was that a malicious user could provoke mkstemp failure
and race to create a symlink with the selected temporary file name,
thus causing the qemu process (usually root owned) to open through
the symlink, overwriting an attacker-chosen file.
This addresses CVE-2012-2652.
http://bugzilla.redhat.com/CVE-2012-2652
Reviewed-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com>
Signed-off-by: Jim Meyering <meyer...@redhat.com>
Signed-off-by: Anthony Liguori <aligu...@us.ibm.com>
diff --git a/block.c b/block.c
index af2ab4f..7547051 100644
--- a/block.c
+++ b/block.c
@@ -409,28 +409,36 @@ int bdrv_create_file(const char* filename,
QEMUOptionParameter *options)
return bdrv_create(drv, filename, options);
}
-#ifdef _WIN32
-void get_tmp_filename(char *filename, int size)
+/*
+ * Create a uniquely-named empty temporary file.
+ * Return 0 upon success, otherwise a negative errno value.
+ */
+int get_tmp_filename(char *filename, int size)
{
+#ifdef _WIN32
char temp_dir[MAX_PATH];
-
- GetTempPath(MAX_PATH, temp_dir);
- GetTempFileName(temp_dir, "qem", 0, filename);
-}
+ /* GetTempFileName requires that its output buffer (4th param)
+ have length MAX_PATH or greater. */
+ assert(size >= MAX_PATH);
+ return (GetTempPath(MAX_PATH, temp_dir)
+ && GetTempFileName(temp_dir, "qem", 0, filename)
+ ? 0 : -GetLastError());
#else
-void get_tmp_filename(char *filename, int size)
-{
int fd;
const char *tmpdir;
- /* XXX: race condition possible */
tmpdir = getenv("TMPDIR");
if (!tmpdir)
tmpdir = "/tmp";
- snprintf(filename, size, "%s/vl.XXXXXX", tmpdir);
+ if (snprintf(filename, size, "%s/vl.XXXXXX", tmpdir) >= size) {
+ return -EOVERFLOW;
+ }
fd = mkstemp(filename);
- close(fd);
-}
+ if (fd < 0 || close(fd)) {
+ return -errno;
+ }
+ return 0;
#endif
+}
/*
* Detect host devices. By convention, /dev/cdrom[N] is always
@@ -753,7 +761,10 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
bdrv_delete(bs1);
- get_tmp_filename(tmp_filename, sizeof(tmp_filename));
+ ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
+ if (ret < 0) {
+ return ret;
+ }
/* Real path is meaningless for protocols */
if (is_protocol)
diff --git a/block/vvfat.c b/block/vvfat.c
index 2dc9d50..0fd3367 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2808,7 +2808,12 @@ static int enable_write_target(BDRVVVFATState *s)
array_init(&(s->commits), sizeof(commit_t));
s->qcow_filename = g_malloc(1024);
- get_tmp_filename(s->qcow_filename, 1024);
+ ret = get_tmp_filename(s->qcow_filename, 1024);
+ if (ret < 0) {
+ g_free(s->qcow_filename);
+ s->qcow_filename = NULL;
+ return ret;
+ }
bdrv_qcow = bdrv_find_format("qcow");
options = parse_option_parameters("", bdrv_qcow->create_options, NULL);
diff --git a/block_int.h b/block_int.h
index b80e66d..3d4abc6 100644
--- a/block_int.h
+++ b/block_int.h
@@ -335,7 +335,7 @@ struct BlockDriverState {
BlockJob *job;
};
-void get_tmp_filename(char *filename, int size);
+int get_tmp_filename(char *filename, int size);
void bdrv_set_io_limits(BlockDriverState *bs,
BlockIOLimit *io_limits);
live well,
vagrant
--- End Message ---
