From eba25057b9a5e19d10ace2bc7716667a31297169 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 28 May 2012 09:27:54 +0200 Subject: 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 Signed-off-by: Jim Meyering Signed-off-by: Anthony Liguori --- block/vvfat.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'block/vvfat.c') diff --git a/block/vvfat.c b/block/vvfat.c index 2dc9d50888..0fd3367d82 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); -- cgit v1.2.3