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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org