GNOME Bugzilla – Bug 342111
message format string vulnerability
Last modified: 2006-05-27 13:03:44 UTC
Steps to reproduce: 1. touch %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s.bmp 2. dia %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s.bmp The issue is public: http://kandangjamur.net/tutorial/dia.txt and vuln-dev@securityfocus.com Stack trace: Other information: I can confirm the problem, but I cannot confirm original report analysis: vsprintf (buf, fmt, *args2) correctly returns: buf = (gchar *) 0xebb4a0 "Failed to load:\nImage file '%p%p%p%p.bmp' contains no data" The real problem seems to be message_create_dialog(), which provides the string message_format to gtk_message_dialog_new(), which is defined as GtkWidget* gtk_message_dialog_new (GtkWindow *parent, GtkDialogFlags flags, GtkMessageType type, GtkButtonsType buttons, const gchar *message_format, ...); Affected are all versions except the old ones using gtk_label_new() I am not sure about correctness of alloc = nearest_pow (MAX(len + 1, 1024)); Maybe 1024 should be MAXPATHLEN. And I don't know, why exactly there are two variables with the same varargs contents - one is analysed to get the proper length, one is used for formatting.
Created attachment 65665 [details] [review] dia-message-format.patch Proposed patch, which seems to fix the bug.
Hi, I'm the Fedora dia package maintainer, as such I've taken a look into this. This is a classic format string vulnerability. At a first glance the fix seems correct and after carefull inspection the fix still seems correct. As such I'm pushing new Fedora Extras packages with this fix included. Thanks Stanislav.
This is CVE-2006-2480.
Created attachment 66038 [details] [review] Patch fixing many format string vulnerabilities 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@suse.cz>. 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.
Created attachment 66086 [details] [review] Patch fixing format string vulnerabilities I was a bit short on time when I mailed my previous mail on this, so I didn't test (I didn't even compile) the patch. It turns out my previous patch contained one cut and paste error causing compilation to fail. The attached patch fixes this and has been tested. Also Stanislav Brabec <sbrabec@suse.cz> has found one potential issue missed: --- plug-ins/xfig/xfig-export.c +++ plug-ins/xfig/xfig-export.c @@ -263,7 +263,7 @@ figWarn(XfigRenderer *renderer, int warning) { if (renderer->warnings[warning]) { - message_warning(renderer->warnings[warning]); + message_warning("%s", renderer->warnings[warning]); renderer->warnings[warning] = NULL; } } Luckily this one isn't a security issue though. There is only one type of warning in the renderer->warnings array and this always gets initialised to: _("No more user-definable colors - using black") and is never changed, so this isn't a problem. Still I missed it :| Notice that it still might be a good idea to fix this too and that the attached patch doesn't fix this (The above diff does).
Patch applied, thank you for your investigation!
Also applied to HEAD: 2006-05-27 Hans Breuer <hans@breuer.org> * app/app_procs.c app/display.c app/filedlg.c app/interface.c app/load_save.c app/sheets.c lib/dia_image.c lib/message.c plug-ins/python/diamodule.c plug-ins/python/pydia-error.c plug-ins/wmf/wmf.cpp : fixed format string vulnerability on HEAD as well. Patch from Hans de Goede, bug #342111