On 05/14/2010 04:08 AM, Ryota Ozaki wrote:
The code specifies driver->cacheDir as the format string,
but it usually doesn't contain '%s', so the subsequent
argument, "/qemu.mem.XXXXXX", is always ignored.
The patch fixes the misuse.
---
src/qemu/qemu_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bb1079e..843f827 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9033,7 +9033,7 @@ qemudDomainMemoryPeek (virDomainPtr dom,
goto endjob;
}
- if (virAsprintf(&tmp, driver->cacheDir, "/qemu.mem.XXXXXX") <
0) {
+ if (virAsprintf(&tmp, "%s/qemu.mem.XXXXXX", driver->cacheDir) <
0) {
ACK. Even worse, if driver->cacheDir contains %n, we have an
exploitable security hole.
Why didn't gcc -Wformat catch this one? Oh, because it doesn't warn on
non-literal formats. So why didn't -Wformat-nonliteral catch it? Oh,
because we don't turn it on, since we have other (provably safe)
non-literals that would trip it up. Maybe it's time to play with the
appropriate '#pragma gcc' to temporarily disable -Wformat-nonliteral
around just the places audited to be safe, if we detect at configure
time that we have new-enough gcc?
Pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org