[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@xxxxxxxxxx>
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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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@xxxxxxxxxx>
+
+ 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@xxxxxxxxxx>
* 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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list