[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-multimedia
mailing list