Hi.

Using "printf(string);" is dangerous, might lead to bugs and even
security issues. If the string being printed contains the "%" character
one can do really dangerous things. Even if you think the string in
question might not be dangerous, future code changes might lead that
piece of code to bugs or security holes.

Some distributions (like Mandriva) even compile all their sources with
the "-Wformat -Werror=format-security" GCC flag.

In most cases the patch is very simple: just replace "printf(string)"
with "printf("%s", string)". This is the case of Groff =)

The attached patch was made for the 1.20.1 version but also applies to
the current cvs checkout.

Some references:
http://wiki.mandriva.com/en/Development/Packaging/Problems#format_not_a_string_literal_and_no_format_arguments
http://wiki.debian.org/Hardening#DEBBUILDHARDENINGFORMAT.28gcc.2BAC8-g.2B-.2B--Wformat-Wformat-security.29
http://en.wikipedia.org/wiki/Format_string_attack

Thanks,
Paulo.

diff -Nru groff-1.20.1/src/devices/grohtml/post-html.cpp new/src/devices/grohtml/post-html.cpp
--- groff-1.20.1/src/devices/grohtml/post-html.cpp	2009-01-09 12:25:52.000000000 -0200
+++ new/src/devices/grohtml/post-html.cpp	2009-10-30 18:02:18.000000000 -0200
@@ -1588,7 +1588,7 @@
 
 	  buffer += as_string(h);
 	  buffer += '\0';
-	  fprintf(f, buffer.contents());
+	  fprintf(f, "%s", buffer.contents());
 	} else
 	  fputs(g->text_string, f);
 	h++;
diff -Nru groff-1.20.1/src/roff/troff/node.cpp new/src/roff/troff/node.cpp
--- groff-1.20.1/src/roff/troff/node.cpp	2009-01-09 12:25:52.000000000 -0200
+++ new/src/roff/troff/node.cpp	2009-10-30 17:58:51.000000000 -0200
@@ -2196,7 +2196,7 @@
   if (c)
     fprintf(stderr, "%c", c);
   else
-    fprintf(stderr, ci->nm.contents());
+    fprintf(stderr, "%s", ci->nm.contents());
   if (push_state)
     fprintf(stderr, " <push_state>");
   if (state)
diff -Nru groff-1.20.1/src/utils/hpftodit/hpftodit.cpp new/src/utils/hpftodit/hpftodit.cpp
--- groff-1.20.1/src/utils/hpftodit/hpftodit.cpp	2009-01-09 12:25:52.000000000 -0200
+++ new/src/utils/hpftodit/hpftodit.cpp	2009-10-30 18:05:13.000000000 -0200
@@ -870,9 +870,9 @@
       else if (!all_flag)
 	continue;
       else if (tfm_type == MSL)
-	printf(hp_msl_to_ucode_name(charcode));
+	printf("%s", hp_msl_to_ucode_name(charcode));
       else
-	printf(unicode_to_ucode_name(charcode));
+	printf("%s", unicode_to_ucode_name(charcode));
 
       printf("\t%d,%d",
 	     scale(char_table[i].width), scale(char_table[i].ascent));
_______________________________________________
bug-groff mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-groff

Reply via email to