[PATCH] artsd security: sprintf

Stefan Westerfeld stefan at space.twc.de
Fri Aug 9 10:02:19 BST 2002


   Hi!

In the process of the artsd DOS attack discussion some people did (rightfully
so) complain that artsd uses sprintf for formatting its debugging output. I
have now replaced this with a variant of g_strdup_(v)printf so I suppose that
the issue is resolved with that. If anybody with high enough paranoid level
for security reviews feels like reading the patch, here it is ;).

I omitted the lengthy implementation of arts_strdup_(v)printf, but you can
find it in the CVS.

   Cu... Stefan
-- 
  -* Stefan Westerfeld, stefan at space.twc.de (PGP!), Hamburg/Germany
     KDE Developer, project infos at http://space.twc.de/~stefan/kde *-         
-------------- next part --------------
Index: debug.cc
===================================================================
RCS file: /home/kde/arts/mcop/debug.cc,v
retrieving revision 1.6
diff -u -b -r1.6 debug.cc
--- debug.cc	2001/07/25 10:41:35	1.6
+++ debug.cc	2002/08/09 09:50:25
@@ -1,8 +1,11 @@
     /*
 
-    Copyright (C) 2000 Stefan Westerfeld
+    Copyright (C) 2000-2002 Stefan Westerfeld
                        stefan at space.twc.de
 
+	(see also below for details on the copyright of arts_strdup_printf,
+	 which is taken from GLib)
+ 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
@@ -33,6 +36,9 @@
 static char *messageAppName = 0;
 static Arts::Mutex *arts_debug_mutex = 0;
 
+/* routines for variable length sprintf without buffer overflow (from GLib) */
+static char* arts_strdup_vprintf(const char *format, va_list args1);
+
 namespace Arts {
 
 /*
@@ -42,8 +48,8 @@
  * Note that the external application is run in the background to
  * avoid blocking the sound server.
  */
-void output_message(Debug::Level level, const char *msg) {
-	char buff[1024];
+static void output_message(Debug::Level level, const char *msg) {
+	char *buff = 0;
 
 	/* default to text output if no message app is defined or if it is a debug message. */
 	if (messageAppName == 0 || !strcmp(messageAppName, "") || (level == Debug::lDebug))
@@ -54,18 +60,22 @@
 
 	switch (level) {
 	  case Debug::lFatal:
-		  sprintf(buff, "%s -e \"Sound server fatal error:\n\n%s\" &", messageAppName, msg);
+		  buff = arts_strdup_printf("%s -e \"Sound server fatal error:\n\n%s\" &", messageAppName, msg);
 		  break;
 	  case Debug::lWarning:
-		  sprintf(buff, "%s -w \"Sound server warning message:\n\n%s\" &", messageAppName, msg);
+		  buff = arts_strdup_printf("%s -w \"Sound server warning message:\n\n%s\" &", messageAppName, msg);
 		  break;
 	  case Debug::lInfo:
-		  sprintf(buff, "%s -i \"Sound server informational message:\n\n%s\" &", messageAppName, msg);
+		  buff = arts_strdup_printf("%s -i \"Sound server informational message:\n\n%s\" &", messageAppName, msg);
 		  break;
 	  default:
 		  break; // avoid compile warning
 	}
+	if(buff != 0)
+	{
 	system(buff);
+		free(buff);
+	}
 }
 
 /*
@@ -76,7 +86,7 @@
  * previously repeated message (if any) and reset the last message and
  * count.
  */
-void display_message(Debug::Level level, const char *msg) {
+static void display_message(Debug::Level level, const char *msg) {
 	static char lastMsg[1024];
 	static Debug::Level lastLevel;
 	static int msgCount = 0;
@@ -90,9 +100,10 @@
 	} else {
 		if (msgCount > 0)
 		{
-			char buff[1024];
-			sprintf(buff, "%s\n(The previous message was repeated %d times.)", lastMsg, msgCount);
+			char *buff;
+			buff = arts_strdup_printf("%s\n(The previous message was repeated %d times.)", lastMsg, msgCount);
 			output_message(lastLevel, buff);
+			free(buff);
 		}
 		strncpy(lastMsg, msg, 1024);
 		lastLevel = level;
@@ -140,12 +151,15 @@
 
 void Arts::Debug::fatal(const char *fmt, ...)
 {
-	char buff[1024];
+	char *buff;
     va_list ap;
+
     va_start(ap, fmt);
-	vsprintf(buff, fmt, ap);
+	buff = arts_strdup_vprintf(fmt, ap);
     va_end(ap);
+
 	display_message(Debug::lFatal, buff);
+	free(buff);
 
 	if(arts_debug_abort) abort();
 	exit(1);
@@ -155,12 +169,15 @@
 {
 	if(lWarning >= arts_debug_level)
 	{
-		char buff[1024];
+		char *buff;
 		va_list ap;
+
 		va_start(ap, fmt);
-		vsprintf(buff, fmt, ap);
+		buff = arts_strdup_vprintf(fmt, ap);
 		va_end(ap);
+
 		display_message(Debug::lWarning, buff);
+		free(buff);
 	}
 }
 
@@ -168,12 +185,15 @@
 {
 	if(lInfo >= arts_debug_level)
 	{
-		char buff[1024];
+		char *buff;
 		va_list ap;
+
 		va_start(ap, fmt);
-		vsprintf(buff, fmt, ap);
+		buff = arts_strdup_vprintf(fmt, ap);
 		va_end(ap);
+
 		display_message(Debug::lInfo, buff);
+		free(buff);
 	}
 }
 
@@ -181,12 +201,15 @@
 {
 	if(lDebug >= arts_debug_level)
 	{
-		char buff[1024];
+		char *buff;
 		va_list ap;
+
 		va_start(ap, fmt);
-		vsprintf(buff, fmt, ap);
+		buff = arts_strdup_vprintf(fmt, ap);
 		va_end(ap);
+
 		display_message(Debug::lDebug, buff);
+		free(buff);
 	}
 }
 
Index: debug.h
===================================================================
RCS file: /home/kde/arts/mcop/debug.h,v
retrieving revision 1.7
diff -u -b -r1.7 debug.h
--- debug.h	2002/03/08 20:30:19	1.7
+++ debug.h	2002/08/09 09:50:25
@@ -104,6 +104,8 @@
 
 #endif
 
+char* arts_strdup_printf (const char *format, ...);
+
 namespace Arts {
 	class Debug {
 	public:
Index: dispatcher.cc
===================================================================
RCS file: /home/kde/arts/mcop/dispatcher.cc,v
retrieving revision 1.65
diff -u -b -r1.65 dispatcher.cc
--- dispatcher.cc	2002/03/15 13:07:19	1.65
+++ dispatcher.cc	2002/08/09 09:50:27
@@ -795,10 +795,12 @@
 
 void Dispatcher::generateServerID()
 {
-	char buffer[4096];
-	sprintf(buffer,"%s-%04x-%08lx",MCOPUtils::getFullHostname().c_str(),
+	char *buffer;
+	buffer = arts_strdup_printf("%s-%04x-%08lx",
+								MCOPUtils::getFullHostname().c_str(),
 									getpid(),time(0));
 	serverID = buffer;
+	free(buffer);
 }
 
 string Dispatcher::objectToString(long objectID)
Index: namedstore.h
===================================================================
RCS file: /home/kde/arts/mcop/namedstore.h,v
retrieving revision 1.4
diff -u -b -r1.4 namedstore.h
--- namedstore.h	2002/03/15 13:07:19	1.4
+++ namedstore.h	2002/08/09 09:50:27
@@ -114,9 +114,9 @@
 				return xname;
 			}
 
-			char buffer[4096];
-			sprintf(buffer,"%s%d",name.c_str(),append++);
-			xname = buffer;
+			char buffer[1024];
+			sprintf(buffer,"%d",append++);
+			xname = name + string(buffer);
 		}
 	}
 };


More information about the kde-core-devel mailing list