132 lines
4.3 KiB
Diff
132 lines
4.3 KiB
Diff
From 2d96e0d8673003d961c1c7b01c26a2fe60ca5bdd Mon Sep 17 00:00:00 2001
|
|
From: Jim Meyering <jim@meyering.net>
|
|
Date: Mon, 28 May 2012 09:27:54 +0200
|
|
Subject: [PATCH] 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
|
|
|
|
Signed-off-by: Jim Meyering <meyering@redhat.com>
|
|
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
(cherry picked from commit c2d76497b6eafcaedc806e07804e7bed55a98a0b)
|
|
---
|
|
block.c | 37 ++++++++++++++++++++++++-------------
|
|
block/vvfat.c | 7 ++++++-
|
|
block_int.h | 2 +-
|
|
3 files changed, 31 insertions(+), 15 deletions(-)
|
|
|
|
diff --git a/block.c b/block.c
|
|
index d015887..b752e84 100644
|
|
--- a/block.c
|
|
+++ b/block.c
|
|
@@ -272,28 +272,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
|
|
@@ -601,7 +609,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 a310ce8..19e3aca 100644
|
|
--- a/block/vvfat.c
|
|
+++ b/block/vvfat.c
|
|
@@ -2799,7 +2799,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 77c0187..415a84a 100644
|
|
--- a/block_int.h
|
|
+++ b/block_int.h
|
|
@@ -238,7 +238,7 @@ struct BlockDriverAIOCB {
|
|
BlockDriverAIOCB *next;
|
|
};
|
|
|
|
-void get_tmp_filename(char *filename, int size);
|
|
+int get_tmp_filename(char *filename, int size);
|
|
|
|
void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs,
|
|
BlockDriverCompletionFunc *cb, void *opaque);
|