Hi all,
A format string vulnerability in dia was reported in CVE-2006-2480, this
has lead me to taking a closer look at the use of formatstrings in dia.
Yesterday I checked all the uses of:
dia's message* funcs
g_print
g_message
g_warning
dia_assert_true
And reported my findings to John Bressers (from RedHat) and Stanislav
Brabec <sbrabec@xxxxxxx>. John has assigned CVE-2006-2453 for the
additonal problems I found.
This morning I also checked (and found issues and fixed) all the uses of:
gtk_message_dialog_new
gtk_message_dialog_format_secondary_text
g_error
I've attached a patch fixing all issues I found. New as of this morning
are the changes / fixes to:
app/display.c
app/filedlg.c
Regards,
Hans
p.s.
There could still be other vararg printf like functions in dia which I
didn't check. I'm in no way claiming this work is complete. With that
said I'm not planning on doing any more auditing for printf like
functions in dia in the near future.
Only in dia-0.95/app: .#filedlg.c
diff -ur dia-0.95.orig/app/app_procs.c dia-0.95/app/app_procs.c
--- dia-0.95.orig/app/app_procs.c 2006-02-27 22:29:02.000000000 +0100
+++ dia-0.95/app/app_procs.c 2006-05-22 22:22:37.000000000 +0200
@@ -1175,7 +1175,7 @@
# endif
if (!g_option_context_parse (context, &argc, &argv, &error)) {
if (error) { /* IMO !error here is a bug upstream, triggered with --gdk-debug=updates */
- g_print (error->message);
+ g_print ("%s", error->message);
g_error_free (error);
} else {
g_print ("Invalid option?");
@@ -1273,22 +1273,22 @@
g_print(_("The original author of Dia was:\n\n"));
for (i = 0; i < NUMBER_OF_ORIG_AUTHORS; i++) {
- g_print(authors[i]); g_print("\n");
+ g_print("%s\n", authors[i]);
}
g_print(_("\nThe current maintainers of Dia are:\n\n"));
for (i = NUMBER_OF_ORIG_AUTHORS; i < NUMBER_OF_ORIG_AUTHORS + NUMBER_OF_MAINTAINERS; i++) {
- g_print(authors[i]); g_print("\n");
+ g_print("%s\n", authors[i]);
}
g_print(_("\nOther authors are:\n\n"));
for (i = NUMBER_OF_ORIG_AUTHORS + NUMBER_OF_MAINTAINERS; i < nauthors; i++) {
- g_print(authors[i]); g_print("\n");
+ g_print("%s\n", authors[i]);
}
g_print(_("\nDia is documented by:\n\n"));
for (i = 0; i < ndocumentors; i++) {
- g_print(documentors[i]); g_print("\n");
+ g_print("%s\n", documentors[i]);
}
exit(0);
Only in dia-0.95/app: app_procs.c~
diff -ur dia-0.95.orig/app/display.c dia-0.95/app/display.c
--- dia-0.95.orig/app/display.c 2006-03-20 22:24:26.000000000 +0100
+++ dia-0.95/app/display.c 2006-05-23 09:39:40.000000000 +0200
@@ -1119,7 +1119,6 @@
Diagram *dia;
GtkWidget *dialog, *button;
gchar *fname;
- gchar *msg;
g_return_if_fail(ddisp != NULL);
@@ -1134,10 +1133,6 @@
fname = dia->filename;
if (!fname)
fname = _("<unnamed>");
- msg = g_strdup_printf (
- _("The diagram '%s'\n"
- "has not been saved. Save changes now?"),
- fname);
dialog = gtk_message_dialog_new(GTK_WINDOW (ddisp->shell),
GTK_DIALOG_MODAL,
@@ -1145,8 +1140,9 @@
GTK_BUTTONS_NONE, /* no standard buttons */
_("Closing diagram without saving"),
NULL);
- gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog), msg);
- g_free (msg);
+ gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog),
+ _("The diagram '%s'\n"
+ "has not been saved. Save changes now?"), fname);
gtk_window_set_title (GTK_WINDOW(dialog), _("Close Diagram"));
button = gtk_button_new_from_stock (GTK_STOCK_CANCEL);
Only in dia-0.95/app: display.c~
diff -ur dia-0.95.orig/app/filedlg.c dia-0.95/app/filedlg.c
--- dia-0.95.orig/app/filedlg.c 2006-02-05 14:42:09.000000000 +0100
+++ dia-0.95/app/filedlg.c 2006-05-23 09:41:29.000000000 +0200
@@ -299,7 +299,6 @@
if (stat(filename, &stat_struct) == 0) {
GtkWidget *dialog = NULL;
- char buffer[300];
char *utf8filename = NULL;
if (!g_utf8_validate(filename, -1, NULL)) {
utf8filename = g_filename_to_utf8(filename, -1, NULL, NULL, NULL);
@@ -310,16 +309,15 @@
}
if (utf8filename == NULL) utf8filename = g_strdup(filename);
- g_snprintf(buffer, 300,
- _("The file '%s' already exists.\n"
- "Do you want to overwrite it?"), utf8filename);
- g_free(utf8filename);
dialog = gtk_message_dialog_new (GTK_WINDOW(fs),
GTK_DIALOG_MODAL, GTK_MESSAGE_QUESTION,
GTK_BUTTONS_YES_NO,
_("File already exists"));
- gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog), buffer);
+ gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog),
+ _("The file '%s' already exists.\n"
+ "Do you want to overwrite it?"), utf8filename);
+ g_free(utf8filename);
gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_RESPONSE_YES);
if (gtk_dialog_run (GTK_DIALOG (dialog)) != GTK_RESPONSE_YES) {
@@ -552,17 +550,15 @@
if (stat(filename, &statbuf) == 0) {
GtkWidget *dialog = NULL;
- char buffer[300];
- g_snprintf(buffer, 300,
- _("The file '%s' already exists.\n"
- "Do you want to overwrite it?"), dia_message_filename(filename));
dialog = gtk_message_dialog_new (GTK_WINDOW(fs),
GTK_DIALOG_MODAL | GTK_DIALOG_DESTROY_WITH_PARENT,
GTK_MESSAGE_QUESTION,
GTK_BUTTONS_YES_NO,
_("File already exists"));
- gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog), buffer);
+ gtk_message_dialog_format_secondary_text(GTK_MESSAGE_DIALOG(dialog),
+ "The file '%s' already exists.\n"
+ "Do you want to overwrite it?"), dia_message_filename(filename));
gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_RESPONSE_YES);
if (gtk_dialog_run (GTK_DIALOG (dialog)) != GTK_RESPONSE_YES) {
Only in dia-0.95/app: filedlg.c~
diff -ur dia-0.95.orig/app/interface.c dia-0.95/app/interface.c
--- dia-0.95.orig/app/interface.c 2006-05-22 22:23:52.000000000 +0200
+++ dia-0.95/app/interface.c 2006-05-22 21:59:00.000000000 +0200
@@ -914,7 +914,7 @@
{
Sheet *sheet = get_sheet_by_name(string);
if (sheet == NULL) {
- message_warning(g_strdup_printf(_("No sheet named %s"), string));
+ message_warning(_("No sheet named %s"), string);
} else {
persistence_set_string("last-sheet-selected", string);
fill_sheet_wbox(sheet);
Only in dia-0.95/app: interface.c~
diff -ur dia-0.95.orig/app/load_save.c dia-0.95/app/load_save.c
--- dia-0.95.orig/app/load_save.c 2006-02-11 23:48:06.000000000 +0100
+++ dia-0.95/app/load_save.c 2006-05-22 21:57:39.000000000 +0200
@@ -200,7 +200,7 @@
g_hash_table_foreach(unknown_hash,
GHFuncUnknownObjects,
unknown_str);
- message_error(unknown_str->str);
+ message_error("%s", unknown_str->str);
}
g_hash_table_destroy(unknown_hash);
g_string_free(unknown_str, TRUE);
Only in dia-0.95/app: load_save.c~
diff -ur dia-0.95.orig/app/sheets.c dia-0.95/app/sheets.c
--- dia-0.95.orig/app/sheets.c 2006-02-26 10:52:32.000000000 +0100
+++ dia-0.95/app/sheets.c 2006-05-22 21:56:44.000000000 +0200
@@ -340,7 +340,7 @@
gdk_pixbuf_render_pixmap_and_mask(pixbuf, pixmap, mask, 1.0);
gdk_pixbuf_unref(pixbuf);
} else {
- message_warning (error->message);
+ message_warning ("%s", error->message);
g_error_free (error);
*pixmap = gdk_pixmap_colormap_create_from_xpm_d
(NULL,
Only in dia-0.95/app: sheets.c~
diff -ur dia-0.95.orig/lib/dia_image.c dia-0.95/lib/dia_image.c
--- dia-0.95.orig/lib/dia_image.c 2005-11-03 23:22:03.000000000 +0100
+++ dia-0.95/lib/dia_image.c 2006-05-22 22:06:26.000000000 +0200
@@ -92,7 +92,7 @@
* only if there is something else wrong while loading it.
*/
if (g_file_test(filename, G_FILE_TEST_EXISTS))
- g_warning (error->message);
+ g_warning ("%s", error->message);
g_error_free (error);
return NULL;
}
Only in dia-0.95/lib: dia_image.c~
diff -ur dia-0.95.orig/lib/message.c dia-0.95/lib/message.c
--- dia-0.95.orig/lib/message.c 2006-05-22 22:24:18.000000000 +0200
+++ dia-0.95/lib/message.c 2006-05-22 21:55:02.000000000 +0200
@@ -86,7 +86,7 @@
0, /* GtkDialogFlags */
type,
GTK_BUTTONS_CLOSE,
- buf);
+ "%s", buf);
if (title) {
gchar *real_title;
Only in dia-0.95: log
diff -ur dia-0.95.orig/plug-ins/python/diamodule.c dia-0.95/plug-ins/python/diamodule.c
--- dia-0.95.orig/plug-ins/python/diamodule.c 2005-08-29 07:17:51.000000000 +0200
+++ dia-0.95/plug-ins/python/diamodule.c 2006-05-22 22:04:37.000000000 +0200
@@ -393,11 +393,11 @@
return NULL;
if (0 == type)
- message_notice (text);
+ message_notice ("%s", text);
else if (1 == type)
- message_warning (text);
+ message_warning ("%s", text);
else
- message_error (text);
+ message_error ("%s", text);
Py_INCREF(Py_None);
return Py_None;
Only in dia-0.95/plug-ins/python: diamodule.c~
diff -ur dia-0.95.orig/plug-ins/python/pydia-error.c dia-0.95/plug-ins/python/pydia-error.c
--- dia-0.95.orig/plug-ins/python/pydia-error.c 2005-09-13 22:19:27.000000000 +0200
+++ dia-0.95/plug-ins/python/pydia-error.c 2006-05-22 22:04:02.000000000 +0200
@@ -46,7 +46,7 @@
PyFile_WriteObject (v, ef, 0);
PyTraceBack_Print(tb, ef);
if (((PyDiaError*)ef)->str && popup)
- message_error (((PyDiaError*)ef)->str->str);
+ message_error ("%s", ((PyDiaError*)ef)->str->str);
g_free (sLoc);
Py_DECREF (ef);
Py_XDECREF(exc);
Only in dia-0.95/plug-ins/python: pydia-error.c~
diff -ur dia-0.95.orig/plug-ins/wmf/wmf.cpp dia-0.95/plug-ins/wmf/wmf.cpp
--- dia-0.95.orig/plug-ins/wmf/wmf.cpp 2006-02-20 22:12:02.000000000 +0100
+++ dia-0.95/plug-ins/wmf/wmf.cpp 2006-05-22 22:23:00.000000000 +0200
@@ -223,7 +223,7 @@
va_end (args);
//fprintf(renderer->file, string);
- g_print(string);
+ g_print("%s", string);
g_free(string);
}
Only in dia-0.95/plug-ins/wmf: wmf.cpp~
