[Libvir] PATCH: Avoid format string abuse (also avoids gcc warnings).
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@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.
On Tue, Nov 06, 2007 at 08:31:06PM +0100, Jim Meyering wrote:
This patch was prompted by warnings like this:
util.c:56: warning: format not a string literal and no format arguments
Hmm, what compiler version are you using ? I don't see those warnings when I build. Or did you add extra compiler flags ? If the latter we should make sure they're included in our default flag set so we don't reintroduce similar flaws in the future. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Tue, Nov 06, 2007 at 08:31:06PM +0100, Jim Meyering wrote:
This patch was prompted by warnings like this:
util.c:56: warning: format not a string literal and no format arguments
Hmm, what compiler version are you using ? I don't see those warnings when I build. Or did you add extra compiler flags ? If the latter we should make sure they're included in our default flag set so we don't reintroduce similar flaws in the future.
gcc snapshot build a week or two ago on rawhide, but these options aren't new. I always use -Wformat and -Wformat-security. Here's a patch: * acinclude.m4 (minimum): Add -Wformat and -Wformat-security. diff --git a/acinclude.m4 b/acinclude.m4 index 15bb7ff..1c4051d 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -26,7 +26,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ try_compiler_flags="" ;; minimum) - try_compiler_flags="-Wall $common_flags" + try_compiler_flags="-Wall -Wformat -Wformat-security $common_flags" ;; yes) try_compiler_flags="-Wall -Wmissing-prototypes $common_flags"
Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Tue, Nov 06, 2007 at 08:31:06PM +0100, Jim Meyering wrote:
This patch was prompted by warnings like this:
util.c:56: warning: format not a string literal and no format arguments Hmm, what compiler version are you using ? I don't see those warnings when I build. Or did you add extra compiler flags ? If the latter we should make sure they're included in our default flag set so we don't reintroduce similar flaws in the future.
gcc snapshot build a week or two ago on rawhide, but these options aren't new. I always use -Wformat and -Wformat-security. Here's a patch:
* acinclude.m4 (minimum): Add -Wformat and -Wformat-security.
diff --git a/acinclude.m4 b/acinclude.m4 index 15bb7ff..1c4051d 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -26,7 +26,7 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ try_compiler_flags="" ;; minimum) - try_compiler_flags="-Wall $common_flags" + try_compiler_flags="-Wall -Wformat -Wformat-security $common_flags" ;; yes) try_compiler_flags="-Wall -Wmissing-prototypes $common_flags"
I'm just going to apply this and your other patch, because I always compile with --enable-compile-warnings=error to catch exactly these sorts of regressions / errors, and I wasn't seeing that bug in util.c until you pointed it out. Thanks for contributing! Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
participants (3)
-
Daniel P. Berrange -
Jim Meyering -
Richard W.M. Jones