[LWN Logo]
[LWN.net]
From:	 Leon Harris <leon@quoll.com>
To:	 bugtraq@securityfocus.com
Subject: Vulnerabilities in the Melange Chat Server
Date:	 Sun, 14 Apr 2002 23:47:16 +0800



There are a number of vulnerabilities in the Melange chat system. I sent 
the following email over a month ago to its author, who is not actively
maintaining it. I include some fixes for the problems encountered, but 
caution that I am no longer working on it, and that there are probably 
others. I hope this is of some use to someone. 

Hi Chris.
I have found another remotely exploitable buffer overflow on melange 
that will cause the server to crash. Given that it doesn't come with an 
init script that runs it as an unpriveliged user ( when I first ran it, 
I ran it as root and I should know better) I would like to release this 
to bugtraq.
Since you advertise your users on melange, I have taken the liberty to 
bcc this email to as many of them as I can (those with melange on their 
sites) prior to bugtraq, in order to give them time to respond.

I would like to state now that I think you have done a good job overall 
with melange - it was easy to work with and nice and logically laid out. 
I enjoyed working with it. I think that maybe when it was written, we 
none of us knew so much about security.



Advisory: buffer overflow in melange server 2.02-beta

Melange is a chat system written in C and java which is freely available 
under GPL (http://melange.terminal.at). It is quite a nice system, and 
has been my pleasure to work with it. It was also coded nearly five 
years ago, at a time when people were not quite so security conscious. 
Its author has indicated that he is not currently maintaining it, due to 
other commitments.
Due to the need for logging to access /var/log directory by default, the 
temptation for the lazy admin is to run it as root. I recommend that if 
you must run it, it needs to be chrooted as a low privileged user, and 
the attatched patches applied. Note that this probably doesn't fix all 
possible exploits, just the ones I was able to quickly locate. Note also 
that I consider my C language skills to be crap, or university quality, 
in that I probably could pass a course but try like hell to stay away 
from that lang if I can in real life.

Partial lists of problems - (don't be mean, there are more, I am just 
copying from my lousy notes from over a month ago).
There are numerous calls to unsafe c functions such as strcpy and sprintf, 
some of which can result in core dumps.
.
1) A remote client can crash the chat server by issuing a /yell command 
with an argument over 500 chars in length. Shellcode exploit has not 
been written, but is possible.
eg:
telnet localhost 6666
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
MELANGE> user test test 0 0
/yell 
mmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmmm

server dies.


2) long lines in /etc/melange.conf causes a buffer overflow and dump core.

3) file names longer than 250 chars cause core dump (fix line 52 of main.c)



diff -Naur server/atool.c ../melange-2.02-beta2/server/atool.c
--- server/atool.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/atool.c	Sun Dec  5 22:39:51 1999
@@ -94,7 +94,7 @@
         strcpy(parameter,data);
 
 #ifdef DEBUG
-    snprintf(server.log.txt,sizeof(server.log.txt),"DEBUG (ATOOL): com: <%s> opt: <%s> par: <%s> at slot %d.\r\n",command,option,parameter,sender);
+    sprintf(server.log.txt,"DEBUG (ATOOL): com: <%s> opt: <%s> par: <%s> at slot %d.\r\n",command,option,parameter,sender);
     util_WriteLog(LL_DEBUG);
 #endif
 
diff -Naur server/auth.c ../melange-2.02-beta2/server/auth.c
--- server/auth.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/auth.c	Sun Dec  5 22:40:10 1999
@@ -84,8 +84,6 @@
     if (util_isSet(UNIQUENICKS)==YES) {
 	if ((util_isSet(GUESTLOGIN)==YES)&&(strcasecmp(client->name,"guest")==0)) { 
 	    sprintf(salt,"%d%c",mySlot,0);
-	    if (strlen(salt) + strlen(client->name) > sizeof(client->name))
-		    return(ERR_NAME);
 	    strcat(client->name,salt);
 	}
 	else {
diff -Naur server/chatutil.c ../melange-2.02-beta2/server/chatutil.c
--- server/chatutil.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/chatutil.c	Sun Dec  5 22:40:22 1999
@@ -64,7 +64,7 @@
 	sprintf(txt,MSG_LEAVE,slotID,slot[slotID].user->name);
 	comm_SendChannelBut(SYSMSG,myChannel,slotID,txt);
 	util_WriteMsgLog(txt);
-	strncpy(server.log.txt,txt,sizeof(server.log.txt));
+	strcpy(server.log.txt,txt);
 	util_WriteLog(LL_NORMAL);
     }
     
@@ -134,4 +134,4 @@
     if (slot[mySlot].user!=NULL)
 	free(slot[mySlot].user);
     slot[mySlot].user=NULL;
-}
+}
\ No newline at end of file
diff -Naur server/client.c ../melange-2.02-beta2/server/client.c
--- server/client.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/client.c	Sun Dec  5 22:40:34 1999
@@ -175,9 +175,9 @@
         if (util_GetNextSubString(inBuffer,cmd,MBUFFSIZE)!=OK) 
 	    if ((strlen(inBuffer)>0)&&(strlen(inBuffer)<(MBUFFSIZE-2)))
 		strcpy(cmd,inBuffer);
-        util_GetNextSubString(inBuffer,name,sizeof(client->name));
-        util_GetNextSubString(inBuffer,password,sizeof(client->passwd));
-        util_GetNextSubString(inBuffer,channel,sizeof(client->channel));
+        util_GetNextSubString(inBuffer,name,MBUFFSIZE);
+        util_GetNextSubString(inBuffer,password,MBUFFSIZE);
+        util_GetNextSubString(inBuffer,channel,MBUFFSIZE);
 	if ((strlen(inBuffer)>0)&&(strlen(inBuffer)<(MBUFFSIZE-2)))
 	    strcpy(group,inBuffer);
       
diff -Naur server/commands.c ../melange-2.02-beta2/server/commands.c
--- server/commands.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/commands.c	Sun Dec  5 22:41:05 1999
@@ -135,7 +135,7 @@
     int i;
     char message[500];
     
-    snprintf(message,sizeof(message),"%s ",message1);
+    sprintf(message,"%s ",message1);
     if (strlen(message2)>0)
 	strcat(message,message2);
     for (i=0;i<strlen(message);i++)
@@ -513,7 +513,7 @@
 	}
     }
     sprintf(txt,MSG_NEWNAME,user,slot[user].user->name,myNewNick);
-    strncpy(slot[user].user->name,myNewNick,sizeof(slot[user].user->name));
+    strcpy(slot[user].user->name,myNewNick);
     comm_SendGroupBut(SYSMSG,user,txt);
     sprintf(txt,MSG_YOURNEWNAME,myNewNick,user);
     comm_SendTo(SYSMSG,user,txt);
diff -Naur server/interpret.c ../melange-2.02-beta2/server/interpret.c
--- server/interpret.c	Sat Jan 12 23:12:40 2002
+++ ../melange-2.02-beta2/server/interpret.c	Sun Dec  5 22:41:41 1999
@@ -56,22 +56,22 @@
 
     strcpy(data,util_FitString(data));
 
-    if ( (strlen(data)<2) || (strlen(data) > 500 ) )					/* Can't be a command ! */
+    if (strlen(data)<2)					/* Can't be a command ! */
         return(ERR_ILLEGALCMD);
 
     if (util_GetNextSubString(data,command,MBUFFSIZE)!=OK) 	/* Get command */
 	if ((strlen(data)>0)&&(strlen(data)<(MBUFFSIZE-2)))
-	    strncpy(command,data,sizeof(command));
+	    strcpy(command,data);
     if (util_GetNextSubString(data,option,MBUFFSIZE)!=OK) 	/* Get option */
 	if ((strlen(data)>0)&&(strlen(data)<(MBUFFSIZE-2)))
-	    strncpy(option,data,sizeof(option));			
+	    strcpy(option,data);			
     if ((strlen(data)>0)&&(strlen(data)<(MMAXTXTLEN-MBUFFSIZE)))	/* Get parameter */
-        strncpy(parameter,data,sizeof(parameter));
+        strcpy(parameter,data);
     command[0]='/';
 
 
 #ifdef DEBUG
-    snprintf(server.log.txt,sizeof(server.log.txt),"DEBUG (User): com: <%s> opt: <%s> par: <%s> slot %d !\r\n",command,option,parameter,sender);
+    sprintf(server.log.txt,"DEBUG (User): com: <%s> opt: <%s> par: <%s> slot %d !\r\n",command,option,parameter,sender);
     util_WriteLog(LL_DEBUG);
 #endif
 
diff -Naur server/main.c ../melange-2.02-beta2/server/main.c
--- server/main.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/main.c	Sun Dec  5 22:41:52 1999
@@ -49,12 +49,12 @@
     
     printf ("%s(C) 1998,1999 by Christian Walter, All rights reserved\r\nhttp://melange.terminal.at       Email: chris@terminal.at\r\n\n",PRGVERSION);
     server.port=PORT; 
-    strncpy(server.configFileName,CONFIGFILE,sizeof(server.configFileName));
+    strcpy(server.configFileName,CONFIGFILE);
     for (i=1;i<argc;i++) {
 	if ((strcasecmp(argv[i],"-p")==0)&&((i+1)<argc))
     	    server.port=atoi(argv[i+1]);
 	if ((strcasecmp(argv[i],"-c")==0)&&((i+1)<argc)) 
-    	    strncpy(server.configFileName,argv[i+1],sizeof(server.configFileName));
+    	    strcpy(server.configFileName,argv[i+1]);
     }
     util_ChatInit();
     if (startup_server()!=OK)
diff -Naur server/sysutil.c ../melange-2.02-beta2/server/sysutil.c
--- server/sysutil.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/sysutil.c	Sun Dec  5 22:42:49 1999
@@ -158,7 +158,7 @@
 
 int sys_SendUserUpdate(int myChannel,int user) {
     char buffer[2045];
-    char utxt[MMAXUSERNAMELEN+15];
+    char utxt[MMAXUSERNAMELEN+10];
     int i,g;
 
     atool_sendUpdate("B");
@@ -203,7 +203,7 @@
 
 int sys_SendGroupUpdate(int user) {
     char buffer[2045];
-    char gtxt[MMAXGROUPNAMELEN+25];
+    char gtxt[MMAXGROUPNAMELEN+15];
     grouplist *lp;
 
     atool_sendUpdate("G");
diff -Naur server/util.c ../melange-2.02-beta2/server/util.c
--- server/util.c	Sat Jan 12 23:11:19 2002
+++ ../melange-2.02-beta2/server/util.c	Sun Dec  5 22:43:03 1999
@@ -510,20 +510,17 @@
 void util_ReadConfigFile(char *configFileName) {
     FILE *configFileHandle;
     char tmpString[500],tmpString2[MBUFFSIZE+2],tmpString3[MBUFFSIZE+2];
-    char tmpChar,fmt[10],fmt2[10];
+    char tmpChar;
     unsigned long configFileLength;
     int i,r,g,b;
 
-    sprintf(fmt,"%%%ds", sizeof(tmpString));
-    sprintf(fmt2,"%%%ds",MBUFFSIZE+2);
-
     if ((configFileHandle=fopen(configFileName,"rb"))!=NULL) {
 	fseek(configFileHandle,0,SEEK_END);
         configFileLength=ftell(configFileHandle);
         rewind(configFileHandle);
 	    
         while (ftell(configFileHandle)<configFileLength) {
-           fscanf (configFileHandle,fmt,tmpString);
+           fscanf (configFileHandle,"%s",tmpString);
 	   if ((tmpString[0]=='#')||(tmpString[0]=='[')) {
 	       do {
 		    fscanf(configFileHandle,"%c",&tmpChar);
@@ -532,13 +529,13 @@
 	   }
 	   if ( ((strcasecmp(tmpString,"ALLOW")==0)&&(util_isSet(SECURITYTYP)==YES)) ||
 	        ((strcasecmp(tmpString,"DENY")==0)&&(util_isSet(SECURITYTYP)!=YES)) ) {
-		       fscanf(configFileHandle,fmt,tmpString);
+		       fscanf(configFileHandle,"%s",tmpString);
 		       if (strlen(tmpString)<MMAXHOSTNAMELEN)
 		           util_insertHost(tmpString, 0);
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"PROFILE")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       if (strcasecmp(tmpString,"deny")==0)
 		    util_Set(SECURITYTYP,YES);       
 	       continue;
@@ -548,19 +545,19 @@
 	   if (strcasecmp(tmpString,"KICKOUTTIME")==0)
 	       fscanf(configFileHandle,"%d",&admin.defaultBannTime);
 	   if (strcasecmp(tmpString,"PASSWD")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       if (strlen(tmpString)<MBUFFSIZE)
 	           strcpy(admin.passwd,tmpString);
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"LOGFILE")==0) {
-	        fscanf(configFileHandle,fmt,tmpString);
+	        fscanf(configFileHandle,"%s",tmpString);
 	        if (strlen(tmpString)<MMAXFILENAMELEN)
 		    strcpy(server.log.logfilename,tmpString);
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"MSGLOGFILE")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       server.log.logMessages=YES;
 	       if ((strcasecmp(tmpString,"on")!=0)&&(strlen(tmpString)<MMAXFILENAMELEN)) 
 	           strcpy(server.log.msgfilename,tmpString);
@@ -573,7 +570,7 @@
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"ALLOWCHANNELS")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       if (strcasecmp(tmpString,"no")==0) {
 		   util_Set(ALLOWUSERCHANNELS,NO);       
 		   for(i=2;i<maxChannels;i++) {
@@ -584,7 +581,7 @@
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"ANONYMOUS")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       	   if (strcasecmp(tmpString,"no")==0) {
 		       util_Set(ALLOWANONYMOUSCHANNEL,NO);
 		       channel[ANONYMOUS].locked=PERMANENT;
@@ -592,44 +589,44 @@
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"UNIQUE")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       	   if (strcasecmp(tmpString,"yes")==0) 
 			util_Set(UNIQUENICKS,YES);       
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"CHANGENICKS")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       	   if (strcasecmp(tmpString,"no")==0) 
 			util_Set(ALLOWCHANGENICKS,NO);
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"GUESTLOGIN")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       	   if (strcasecmp(tmpString,"no")==0)
 			util_Set(GUESTLOGIN,NO);       
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"DBAUTH")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       	   if (strcasecmp(tmpString,"yes")==0)
 			util_Set(DBAUTH,YES);       
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"DBINIT")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       	   if (strcasecmp(tmpString,"yes")==0)
 			util_Set(DBINIT,YES);       
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"SHOWHOSTS")==0) {
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       	   if (strcasecmp(tmpString,"no")==0)
 			util_Set(SHOWHOSTS,NO);       
 	       continue;
 	   }
 	   if (strcasecmp(tmpString,"SYSCHANNEL")==0) {
 	       fscanf(configFileHandle,"%d",&i);
-	       fscanf(configFileHandle,fmt,tmpString);
+	       fscanf(configFileHandle,"%s",tmpString);
 	       if ((strlen(tmpString)>2)&&(strlen(tmpString)<MMAXCHANNELNAMELEN)&&(tmpString!=NULL))
 	           if ((i>1)&&(i<maxChannels))
 		       if (channel[i].owner==NOBODY) {
@@ -642,9 +639,9 @@
 	   }
 	   if (strcasecmp(tmpString,"GROUP")==0) {
 	       fscanf(configFileHandle,"%d",&i);
-	       fscanf(configFileHandle,fmt,tmpString);
-	       fscanf(configFileHandle,fmt2,tmpString2);
-	       fscanf(configFileHandle,fmt2,tmpString3);
+	       fscanf(configFileHandle,"%s",tmpString);
+	       fscanf(configFileHandle,"%s",tmpString2);
+	       fscanf(configFileHandle,"%s",tmpString3);
 	       if ((tmpString!=NULL)&&(tmpString2!=NULL)&&(i>0))
 	           util_insertGroup(i,tmpString,tmpString2,tmpString3);
 	       continue;
@@ -659,8 +656,8 @@
 	   }
 	   if (strcasecmp(tmpString,"GRPPERMS")==0) {
 	       fscanf(configFileHandle,"%d",&i);
-	       fscanf(configFileHandle,fmt,tmpString);
-	       fscanf(configFileHandle,fmt2,tmpString2);
+	       fscanf(configFileHandle,"%s",tmpString);
+	       fscanf(configFileHandle,"%s",tmpString2);
 	       if ((tmpString!=NULL)&&(tmpString2!=NULL)&&(i>0)) {
 	           if (strcasecmp(tmpString,"ALLOWED_TO_TALK")==0) {
 		       if (strcasecmp(tmpString2,"no")==0) util_setGroupPerms(i,GRP_ALLOWED_TO_TALK,NO);
@@ -726,7 +723,7 @@
 #endif
 
 void util_PrintConfiguration() {
-    char infoText[300];
+    char infoText[250];
     hostslist *lp;
     
     sprintf(infoText,"--> Using configfile: %s (%d,%x)\r\n",server.configFileName,admin.config,admin.config);