lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri Apr  7 16:08:20 2006
From: adobriyan at gmail.com (Alexey Dobriyan)
Subject: Re: Format string in Doomsday 1.8.6

On Mon, Apr 03, 2006 at 11:20:34PM +0200, Luigi Auriemma wrote:
> Application:  Doomsday engine

> The Doomsday engine contains many functions used for the visualization
> of the messages in the console.
> Both Con_Message and conPrintf are vulnerable to a format string
> vulnerability which could allow an attacker to execute malicious code
> versus the server or the clients.
> The first function calls a "Con_Printf(buffer)" while the second one
> calls a "SW_Printf(prbuff)" if SW_IsActive is enabled (which means
> ever).
>
> >From Src/con_main.c:
>
> void Con_Message(const char *message, ...)
> {
> 	va_list argptr;
> 	char   *buffer;
>
> 	if(message[0])
> 	{
> 		buffer = malloc(0x10000);
>
> 		va_start(argptr, message);
> 		vsprintf(buffer, message, argptr);

Duh!

> 		va_end(argptr);
>
> #ifdef UNIX
> 		if(!isDedicated)
> 		{
> 			// These messages are supposed to be visible in the real console.
> 			fprintf(stderr, "%s", buffer);
> 		}
> #endif
>
> 		// These messages are always dumped. If consoleDump is set,
> 		// Con_Printf() will dump the message for us.
> 		if(!consoleDump)
> 			printf("%s", buffer);
>
> 		// Also print in the console.
> 		Con_Printf(buffer);
>
> 		free(buffer);
> 	}
> 	Con_DrawStartupScreen(true);
> }
>
> ...
>
> void conPrintf(int flags, const char *format, va_list args)
> {
> 	unsigned int i;
> 	int     lbc;				// line buffer cursor
> 	char   *prbuff, *lbuf = malloc(maxLineLen + 1);
> 	cbline_t *line;
>
> 	if(flags & CBLF_RULER)
> 	{
> 		Con_AddRuler();
> 		flags &= ~CBLF_RULER;
> 	}
>
> 	// Allocate a print buffer that will surely be enough (64Kb).
> 	// FIXME: No need to allocate on EVERY printf call!
> 	prbuff = malloc(65536);
>
> 	// Format the message to prbuff.
> 	vsprintf(prbuff, format, args);
>
> 	if(consoleDump)
> 		fprintf(outFile, "%s", prbuff);
> 	if(SW_IsActive())
> 		SW_Printf(prbuff);

> 4) Fix
> ======
>
>
> No fix.

C'mon! Just how hard it to do something like this:

diff -uprN Src.orig/con_main.c Src/con_main.c
--- Src.orig/con_main.c	2005-01-08 19:11:54.000000000 +0300
+++ Src/con_main.c	2006-04-07 18:22:55.000000000 +0400
@@ -988,7 +988,7 @@ static void printcvar(cvar_t *var, char
 	if(var->flags & CVF_PROTECTED)
 		equals = ':';
 
-	Con_Printf(prefix);
+	Con_Printf("%s", prefix);
 	switch (var->type)
 	{
 	case CVT_NULL:
@@ -2782,7 +2782,7 @@ void Con_Message(const char *message, ..
 			printf("%s", buffer);
 
 		// Also print in the console.
-		Con_Printf(buffer);
+		Con_Printf("%s", buffer);
 
 		free(buffer);
 	}
diff -uprN Src.orig/sys_master.c Src/sys_master.c
--- Src.orig/sys_master.c	2004-06-13 16:46:30.000000000 +0400
+++ Src/sys_master.c	2006-04-07 18:23:56.000000000 +0400
@@ -171,7 +171,7 @@ int N_MasterSendAnnouncement(void *parm)
 	   memset(buf, 0, sizeof(buf));
 	   while(recv(s, buf, sizeof(buf) - 1, 0) > 0)
 	   {
-	   Con_Printf(buf);
+	   Con_Printf("%s", buf);
 	   memset(buf, 0, sizeof(buf));
 	   }
 	 */

Or even better:

diff -uprN Src.orig/Common/m_multi.c Src/Common/m_multi.c
--- Src.orig/Common/m_multi.c	2004-10-23 16:42:46.000000000 +0400
+++ Src/Common/m_multi.c	2006-04-07 18:09:21.000000000 +0400
@@ -312,7 +312,7 @@ int Executef(int silent, char *format, .
 	char    buffer[512];
 
 	va_start(argptr, format);
-	vsprintf(buffer, format, argptr);
+	vsnprintf(buffer, sizeof(buffer), format, argptr);
 	va_end(argptr);
 	return Con_Execute(buffer, silent);
 }
diff -uprN Src.orig/Common/p_xgline.c Src/Common/p_xgline.c
--- Src.orig/Common/p_xgline.c	2004-12-23 17:48:56.000000000 +0300
+++ Src/Common/p_xgline.c	2006-04-07 18:09:15.000000000 +0400
@@ -81,7 +81,7 @@ void XG_Dev(const char *format, ...)
 	if(!xgDev)
 		return;
 	va_start(args, format);
-	vsprintf(buffer, format, args);
+	vsnprintf(buffer, sizeof(buffer), format, args);
 	strcat(buffer, "\n");
 	Con_Message(buffer);
 	va_end(args);
diff -uprN Src.orig/con_main.c Src/con_main.c
--- Src.orig/con_main.c	2005-01-08 19:11:54.000000000 +0300
+++ Src/con_main.c	2006-04-07 18:22:55.000000000 +0400
@@ -988,7 +988,7 @@ static void printcvar(cvar_t *var, char 
 	if(var->flags & CVF_PROTECTED)
 		equals = ':';
 
-	Con_Printf(prefix);
+	Con_Printf("%s", prefix);
 	switch (var->type)
 	{
 	case CVT_NULL:
@@ -1304,7 +1304,7 @@ int Con_Executef(int silent, const char 
 	char    buffer[4096];
 
 	va_start(argptr, command);
-	vsprintf(buffer, command, argptr);
+	vsnprintf(buffer, sizeof(buffer), command, argptr);
 	va_end(argptr);
 	return Con_Execute(buffer, silent);
 }
@@ -1966,7 +1966,7 @@ void conPrintf(int flags, const char *fo
 	prbuff = malloc(65536);
 
 	// Format the message to prbuff.
-	vsprintf(prbuff, format, args);
+	vsnprintf(prbuff, sizeof(prbuff), format, args);
 
 	if(consoleDump)
 		fprintf(outFile, "%s", prbuff);
@@ -2765,7 +2765,7 @@ void Con_Message(const char *message, ..
 		buffer = malloc(0x10000);
 
 		va_start(argptr, message);
-		vsprintf(buffer, message, argptr);
+		vsnprintf(buffer, sizeof(buffer), message, argptr);
 		va_end(argptr);
 
 #ifdef UNIX
@@ -2782,7 +2782,7 @@ void Con_Message(const char *message, ..
 			printf("%s", buffer);
 
 		// Also print in the console.
-		Con_Printf(buffer);
+		Con_Printf("%s", buffer);
 
 		free(buffer);
 	}
@@ -2806,7 +2806,7 @@ void Con_Error(const char *error, ...)
 		fprintf(outFile, "Con_Error: Stack overflow imminent, aborting...\n");
 
 		va_start(argptr, error);
-		vsprintf(buff, error, argptr);
+		vsnprintf(buff, sizeof(buff), error, argptr);
 		va_end(argptr);
 		Sys_MessageBox(buff, true);
 
@@ -2821,7 +2821,7 @@ void Con_Error(const char *error, ...)
 	Dir_ChDir(&ddRuntimeDir);
 
 	va_start(argptr, error);
-	vsprintf(err, error, argptr);
+	vsnprintf(err, sizeof(err), error, argptr);
 	va_end(argptr);
 	fprintf(outFile, "%s\n", err);
 
diff -uprN Src.orig/dd_pinit.c Src/dd_pinit.c
--- Src.orig/dd_pinit.c	2004-06-01 20:11:38.000000000 +0400
+++ Src/dd_pinit.c	2006-04-07 18:17:27.000000000 +0400
@@ -82,7 +82,7 @@ void DD_ErrorBox(boolean error, char *fo
 	va_list args;
 
 	va_start(args, format);
-	vsprintf(buff, format, args);
+	vsnprintf(buff, sizeof(buff), format, args);
 	va_end(args);
 #ifdef WIN32
 	MessageBox(NULL, buff, "Doomsday " DOOMSDAY_VERSION_TEXT,
diff -uprN Src.orig/dpMapLoad/glBSP/system.c Src/dpMapLoad/glBSP/system.c
--- Src.orig/dpMapLoad/glBSP/system.c	2004-07-25 00:05:32.000000000 +0400
+++ Src/dpMapLoad/glBSP/system.c	2006-04-07 18:16:37.000000000 +0400
@@ -54,7 +54,7 @@ void FatalError(const char *str, ...)
   va_list args;
 
   va_start(args, str);
-  vsprintf(message_buf, str, args);
+  vsnprintf(message_buf, sizeof(message_buf), str, args);
   va_end(args);
 
   (* cur_funcs->fatal_error)("\nError: *** %s ***\n\n", message_buf);
@@ -68,7 +68,7 @@ void InternalError(const char *str, ...)
   va_list args;
 
   va_start(args, str);
-  vsprintf(message_buf, str, args);
+  vsnprintf(message_buf, sizeof(message_buf), str, args);
   va_end(args);
 
   (* cur_funcs->fatal_error)("\nINTERNAL ERROR: *** %s ***\n\n", message_buf);
@@ -82,7 +82,7 @@ void PrintMsg(const char *str, ...)
   va_list args;
 
   va_start(args, str);
-  vsprintf(message_buf, str, args);
+  vsnprintf(message_buf, sizeof(message_buf), str, args);
   va_end(args);
 
   (* cur_funcs->print_msg)("%s", message_buf);
@@ -100,7 +100,7 @@ void PrintWarn(const char *str, ...)
   va_list args;
 
   va_start(args, str);
-  vsprintf(message_buf, str, args);
+  vsnprintf(message_buf, sizeof(message_buf), str, args);
   va_end(args);
 
   (* cur_funcs->print_msg)("Warning: %s", message_buf);
@@ -122,7 +122,7 @@ void PrintMiniWarn(const char *str, ...)
   if (cur_info->mini_warnings)
   {
     va_start(args, str);
-    vsprintf(message_buf, str, args);
+    vsnprintf(message_buf, sizeof(message_buf), str, args);
     va_end(args);
 
     (* cur_funcs->print_msg)("Warning: %s", message_buf);
@@ -132,7 +132,7 @@ void PrintMiniWarn(const char *str, ...)
 
 #if DEBUG_ENABLED
   va_start(args, str);
-  vsprintf(message_buf, str, args);
+  vsnprintf(message_buf, sizeof(message_buf), str, args);
   va_end(args);
 
   PrintDebug("MiniWarn: %s", message_buf);
diff -uprN Src.orig/dpMapLoad/maploader.c Src/dpMapLoad/maploader.c
--- Src.orig/dpMapLoad/maploader.c	2004-10-23 16:42:48.000000000 +0400
+++ Src/dpMapLoad/maploader.c	2006-04-07 18:17:05.000000000 +0400
@@ -202,7 +202,7 @@ static void fatal_error(const char *str,
 	va_list args;
 
 	va_start(args, str);
-	vsprintf(buffer, str, args);
+	vsnprintf(buffer, sizeof(buffer), str, args);
 	va_end(args);
 
 	Con_Error(buffer);
@@ -218,7 +218,7 @@ static void print_msg(const char *str, .
 	va_list args;
 
 	va_start(args, str);
-	vsprintf(buffer, str, args);
+	vsnprintf(buffer, sizeof(buffer), str, args);
 	va_end(args);
 
 	Con_Message(buffer);
diff -uprN Src.orig/drD3D/main.cpp Src/drD3D/main.cpp
--- Src.orig/drD3D/main.cpp	2004-12-23 18:52:48.000000000 +0300
+++ Src/drD3D/main.cpp	2006-04-07 18:09:43.000000000 +0400
@@ -75,7 +75,7 @@ void DP(const char *format, ...)
 	va_list args;
 	char buf[2048];
 	va_start(args, format);
-	vsprintf(buf, format, args);
+	vsnprintf(buf, sizeof(buf), format, args);
 	va_end(args);
 	Con_Message("%s\n", buf);
 }
diff -uprN Src.orig/m_string.c Src/m_string.c
--- Src.orig/m_string.c	2004-06-01 20:11:40.000000000 +0400
+++ Src/m_string.c	2006-04-07 18:17:58.000000000 +0400
@@ -185,7 +185,7 @@ void Str_Appendf(ddstring_t * ds, const 
 
 	// Print the message into the buffer.
 	va_start(args, format);
-	vsprintf(buf, format, args);
+	vsnprintf(buf, sizeof(buf), format, args);
 	va_end(args);
 	Str_Append(ds, buf);
 }
diff -uprN Src.orig/sys_master.c Src/sys_master.c
--- Src.orig/sys_master.c	2004-06-13 16:46:30.000000000 +0400
+++ Src/sys_master.c	2006-04-07 18:23:56.000000000 +0400
@@ -171,7 +171,7 @@ int N_MasterSendAnnouncement(void *parm)
 	   memset(buf, 0, sizeof(buf));
 	   while(recv(s, buf, sizeof(buf) - 1, 0) > 0)
 	   {
-	   Con_Printf(buf);
+	   Con_Printf("%s", buf);
 	   memset(buf, 0, sizeof(buf));
 	   }
 	 */
diff -uprN Src.orig/sys_musd_win.c Src/sys_musd_win.c
--- Src.orig/sys_musd_win.c	2004-06-01 20:11:50.000000000 +0400
+++ Src/sys_musd_win.c	2006-04-07 18:17:44.000000000 +0400
@@ -779,7 +779,7 @@ int DM_WinCDCommand(char *returnInfo, in
 	MCIERROR error;
 
 	va_start(args, format);
-	vsprintf(buf, format, args);
+	vsnprintf(buf, sizeof(buf), format, args);
 	va_end(args);
 
 	if((error = mciSendString(buf, returnInfo, returnLength, NULL)))
diff -uprN Src.orig/sys_sock.c Src/sys_sock.c
--- Src.orig/sys_sock.c	2004-06-01 20:11:50.000000000 +0400
+++ Src/sys_sock.c	2006-04-07 18:15:13.000000000 +0400
@@ -95,7 +95,7 @@ void N_SockPrintf(socket_t s, const char
 
 	// Print the message into the buffer.
 	va_start(args, format);
-	length = vsprintf(buf, format, args);
+	length = vsnprintf(buf, sizeof(buf), format, args);
 	va_end(args);
 
 	if(length > sizeof(buf))
diff -uprN Src.orig/sys_stwin.c Src/sys_stwin.c
--- Src.orig/sys_stwin.c	2005-01-03 16:08:50.000000000 +0300
+++ Src/sys_stwin.c	2006-04-07 18:13:25.000000000 +0400
@@ -137,7 +137,7 @@ void SW_Printf(const char *format, ...)
 		return;
 
 	va_start(args, format);
-	printedChars += vsprintf(buf, format, args);
+	printedChars += vsnprintf(buf, sizeof(buf), format, args);
 	va_end(args);
 
 	if(printedChars > 32768)


Even more better is to apply rm(1).

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ