[Libvir] PATCH: Avoid format string abuse (also avoids gcc warnings).

[Jim Meyering]

[resending with the patch in-line, not as an attachment --
 mailman removed my attached patch the first time!  Bad mailman.  Bad. ]

This patch was prompted by warnings like this:

  util.c:56: warning: format not a string literal and no format arguments

and they're legitimate.

Imagine a format string contains "%%..." goes through the vnsprintf
call, which reduces it to "%...".  If the result string is then passed
to __virRaiseError as the format string, then *boom*.
Instead, use "%s" as the format, with the non-literal as
the matching argument.  Patch below.

I searched the sources for %% and *did* find one potential problem:

    $ git-grep -B1 %% > k
    po/ms.po-msgid "too many drivers registered in %s"
    po/ms.po:msgstr "terlalu banyak spesifikasi penukaran %% pada suffiks"
    --
    src/xend_internal.c-            case '\n':
    src/xend_internal.c:                snprintf(ptr, 4, "%%%02x", string[i]);

since "% p" does happen to be a valid format string!
So if someone using Malaysian messages provoked that particular
diagnostic in a code path that takes it through __virRaiseError,
bad things might happen.  Big "if", of course :-)  I didn't try.

2007-11-06  Jim Meyering  <meyering at redhat.com>

	Avoid risk of format string abuse (also avoids gcc warnings).
	* src/util.c (ReportError): Use a literal "%s" format string.
	* src/remote_internal.c (server_error): Likewise.
	* src/qemu_conf.c (qemudReportError): Likewise.

-----------------------------
>From 8e0cafec021ba3cbec55a4d691f19c14ed9e587f Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Tue, 6 Nov 2007 20:14:21 +0100
Subject: [PATCH] Avoid risk of format string abuse (also avoids gcc warnings).

* src/util.c (ReportError): Use a literal "%s" format string.
* src/remote_internal.c (server_error): Likewise.
* src/qemu_conf.c (qemudReportError): Likewise.

Signed-off-by: Jim Meyering <meyering at redhat.com>
---
 ChangeLog             |    7 +++++++
 src/qemu_conf.c       |    2 +-
 src/remote_internal.c |    2 +-
 src/util.c            |    2 +-
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 511ed6a..535173d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2007-11-06  Jim Meyering  <meyering at redhat.com>
+
+	Avoid risk of format string abuse (also avoids gcc warnings).
+	* src/util.c (ReportError): Use a literal "%s" format string.
+	* src/remote_internal.c (server_error): Likewise.
+	* src/qemu_conf.c (qemudReportError): Likewise.
+
 Tue Nov  6 17:24:16 CET 2007 Daniel Veillard <veillard at redhat.com>

 	* src/xs_internals.c: patch from Chris Lalancette, forgot to
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 78f4699..3556a9a 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -64,7 +64,7 @@ void qemudReportError(virConnectPtr conn,
         errorMessage[0] = '\0';
     }
     __virRaiseError(conn, dom, net, VIR_FROM_QEMU, code, VIR_ERR_ERROR,
-                    NULL, NULL, NULL, -1, -1, errorMessage);
+                    NULL, NULL, NULL, -1, -1, "%s", errorMessage);
 }

 int qemudLoadDriverConfig(struct qemud_driver *driver,
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 3af326f..1420a88 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -3073,7 +3073,7 @@ server_error (virConnectPtr conn, remote_error *err)
                      err->domain, err->code, err->level,
                      str1, str2, str3,
                      err->int1, err->int2,
-                     message);
+                     "%s", message);
 }

 /* get_nonnull_domain and get_nonnull_network turn an on-wire
diff --git a/src/util.c b/src/util.c
index eb57859..c964a63 100644
--- a/src/util.c
+++ b/src/util.c
@@ -53,7 +53,7 @@ ReportError(virConnectPtr conn,
         errorMessage[0] = '\0';
     }
     __virRaiseError(conn, dom, net, VIR_FROM_NONE, code, VIR_ERR_ERROR,
-                    NULL, NULL, NULL, -1, -1, errorMessage);
+                    NULL, NULL, NULL, -1, -1, "%s", errorMessage);
 }

 static int virSetCloseExec(int fd) {
--
1.5.3.5.561.g140d