[Qemu-devel] String format vulnerability discovered in monitor.c

[Qemu-devel] String format vulnerability discovered in monitor.c

[Tim]

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

This evening, I grepped the codebase for tell-tale signs of string
format vulnerabilities.  I found only one instance of the printf family
of functions which appears to have the problem.  Which if you ask me, is
pretty good.  There may be others I missed, but I figured I would submit
this patch before I look further.

The problem exists at the (qemu) prompt when a user uses the arrow keys
to retrieve commands typed previously.  The code, annotated:

--SNIP--
static void term_print_cmdline (const char *cmdline)
{
    term_show_prompt();
    term_printf(cmdline);  /* <---- problem here */
    term_flush();
}
--SNIP--

The term_printf() function eventually calls vprintf().  This wrapper
function has a similar parameter syntax, which means the first parameter
passed to it is treated as the formatting string.  The problem can be
demostrated trivially at the (qemu) prompt:

--SNIP--
QEMU 0.5.5 monitor - type 'help' for more information
(qemu) foo %x
unknown command: 'foo'
(qemu) 
(qemu) foo 80b860c
--SNIP--

The last line you see is where I hit up arrow.  Instead of printing the
proper 'foo %x', it printed the hexadecimal representation of the next
chunk of memory (what it expect to be in the vprintf() parameter list,
but isn't).  This bug can be exploited to execute arbitrary code:
  http://www.hackerscenter.com/articles/Article.asp?id=52

This of course has little impact on a default install, but I wouldn't be
surprised if users (or if later on, *ix distributions) granted
additional privileges to the qemu binary via sudo or suid/sgid bits, in
order to have direct access to hardware devices.  A bug like this would
then allow privilege escalation locally.

Once again, not a big deal, but something to keep an eye out for.
Trivial patch is attached.

cheers,
tim


[-- Attachment #2: monitor.patch --]
[-- Type: text/plain, Size: 292 bytes --]

--- monitor.c	2004-06-13 21:08:13.000000000 -0700
+++ monitor-new.c	2004-06-13 21:07:43.000000000 -0700
@@ -1437,7 +1437,7 @@
 static void term_print_cmdline (const char *cmdline)
 {
     term_show_prompt();
-    term_printf(cmdline);
+    term_printf("%s", cmdline);
     term_flush();
 }