[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(a)redhat.com&gt; 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: Jim Meyering <meyering(a)redhat.com&gt; 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(a)redhat.com&gt; --- 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(a)redhat.com&gt; + + 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(a)redhat.com&gt; * 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