* Corrected string lengths according to bugreport:198

- CHATBOX_SIZE: 70 -> 70+1
- removed some too aggressive checks in clif_parse_globalmessage()
- removed CHAT_SIZE define as it actually doesn't apply anywhere
- added CHAT_SIZE_MAX to serve as a custom limit to input string lengths
- added length/contents checks to /b and /lb (against fake names)

git-svn-id: https://rathena.svn.sourceforge.net/svnroot/rathena/trunk@11386 54d463be-8e91-2dee-dedb-b68131a5f0ec

1 files changed, 92 insertions, 83 deletions

diff --git a/src/map/clif.c b/src/map/clif.c
index 3e7919159..379f2829b 100644
--- a/src/map/clif.c
+++ b/src/map/clif.c
@@ -6923,12 +6923,11 @@ void clif_emotion(struct block_list *bl,int type)
 void clif_talkiebox(struct block_list* bl, const char* talkie)
 {
 	unsigned char buf[86];
-
 	nullpo_retv(bl);
 
-	WBUFW(buf,0)=0x191;
-	WBUFL(buf,2)=bl->id;
-	memcpy(WBUFP(buf,6),talkie,MESSAGE_SIZE);
+	WBUFW(buf,0) = 0x191;
+	WBUFL(buf,2) = bl->id;
+	safestrncpy((char*)WBUFP(buf,6),talkie,MESSAGE_SIZE);
 	clif_send(buf,packet_len(0x191),bl,AREA);
 }
 
@@ -8212,7 +8211,7 @@ void clif_parse_GetCharNameRequest(int fd, struct map_session_data *sd)
 
 /*==========================================
  * Validates and processes global messages
- * S 008c/00f3 <packet len>.w <text>.?B (<name> : <message>)
+ * S 008c/00f3 <packet len>.w <text>.?B (<name> : <message>) 00
  *------------------------------------------*/
 void clif_parse_GlobalMessage(int fd, struct map_session_data* sd)
 {
@@ -8223,12 +8222,12 @@ void clif_parse_GlobalMessage(int fd, struct map_session_data* sd)
 	// basic structure checks
 	if( packetlen > RFIFOREST(fd) )
 	{	// there has to be enough data to read
-		ShowWarning("clif_parse_GlobalMessage: Received malformed packet from player '%s' (length is incorrect)!", sd->status.name);
+		ShowWarning("clif_parse_GlobalMessage: Received malformed packet from player '%s' (length is incorrect)!\n", sd->status.name);
 		return;
 	}
 	if( packetlen < 4 + 1 )
 	{	// 4-byte header and at least an empty string is expected
-		ShowWarning("clif_parse_GlobalMessage: Received malformed packet from player '%s' (no message data)!", sd->status.name);
+		ShowWarning("clif_parse_GlobalMessage: Received malformed packet from player '%s' (no message data)!\n", sd->status.name);
 		return;
 	}
 
@@ -8242,41 +8241,31 @@ void clif_parse_GlobalMessage(int fd, struct map_session_data* sd)
 		name[namelen] != ' ' || name[namelen+1] != ':' || name[namelen+2] != ' ' ) // followed by ' : '
 	{
 		//Hacked message, or infamous "client desynch" issue where they pick one char while loading another.
-		ShowWarning("clif_parse_GlobalMessage: Player '%s' sent a message using an incorrect name! Forcing a relog...", sd->status.name);
+		ShowWarning("clif_parse_GlobalMessage: Player '%s' sent a message using an incorrect name! Forcing a relog...\n", sd->status.name);
 		clif_setwaitclose(fd); // Just kick them out to correct it.
 		return;
 	}
 
 	message = text + namelen + 3;
-	messagelen = textlen - namelen - 3 - 1; // this should be the message length
+	messagelen = textlen - namelen - 3; // this should be the message length (w/ zero byte included)
 	// verify <message> part of the packet
-	if( message[messagelen] != '\0' )
+	if( message[messagelen-1] != '\0' )
 	{	// message must be zero-terminated
-		ShowWarning("clif_parse_GlobalMessage: Player '%s' sent an unterminated string!", sd->status.name);
+		ShowWarning("clif_parse_GlobalMessage: Player '%s' sent an unterminated string!\n", sd->status.name);
 		return;		
 	}
-	if( messagelen != strnlen(message, messagelen+1) )
+	if( messagelen != strnlen(message, messagelen)+1 )
 	{	// the declared length must match real length
-		ShowWarning("clif_parse_GlobalMessage: Received malformed packet from player '%s' (length is incorrect)!", sd->status.name);
+		ShowWarning("clif_parse_GlobalMessage: Received malformed packet from player '%s' (length is incorrect)!\n", sd->status.name);
 		return;		
 	}
-	if( messagelen > CHATBOX_SIZE )
+	if( messagelen > CHAT_SIZE_MAX )
 	{	// messages mustn't be too long
-		int i;
-		// special case here - allow some more freedom for frost joke & dazzler 
-		// TODO:? You could use a state flag when FrostJoke/Scream is used, and unset it once the skill triggers. [Skotlex]
-		ARR_FIND( 0, MAX_SKILLTIMERSKILL, i, sd->ud.skilltimerskill[i] == 0 || sd->ud.skilltimerskill[i]->skill_id == BA_FROSTJOKE || sd->ud.skilltimerskill[i]->skill_id == DC_SCREAM );
-
-		if( i == MAX_SKILLTIMERSKILL || !sd->ud.skilltimerskill[i])
-		{	// normal message, too long
-			ShowWarning("clif_parse_GlobalMessage: Player '%s' sent a message too long ('%.*s')!", sd->status.name, CHATBOX_SIZE, message);
-			return;
-		}
-		if( messagelen > 255 )
-		{	// frost joke/dazzler, but still too long
-			ShowWarning("clif_parse_GlobalMessage: Player '%s' sent a message too long ('%.*s')!", sd->status.name, 255, message);
-			return;
-		}
+		// Normally you can only enter CHATBOX_SIZE-1 chars into the chat box, but Frost Joke / Dazzler's text can be longer.
+		// Neither the official client nor server place any restriction on the length of the text in the packet,
+		// but we'll only allow reasonably long strings here. This also makes sure all strings fit into the `chatlog` table.
+		ShowWarning("clif_parse_GlobalMessage: Player '%s' sent a message too long ('%.*s')!\n", sd->status.name, CHAT_SIZE_MAX, message);
+		return;
 	}
 
 	if( is_atcommand(fd, sd, text) != AtCommand_None || is_charcommand(fd, sd, text) != CharCommand_None )
@@ -8771,26 +8760,40 @@ void clif_parse_Wis(int fd, struct map_session_data* sd)
 
 /*==========================================
  * /b
+ * S 0099 <packet len>.w <text>.?B (<name>: <message>) 00
  *------------------------------------------*/
-void clif_parse_GMmessage(int fd, struct map_session_data *sd)
+void clif_parse_GMmessage(int fd, struct map_session_data* sd)
 {
-	char* mes;
-	int size, lv;
+	char *text, *name, *message;
+	unsigned int textlen, namelen, messagelen;
+	int lv;
 
 	if (battle_config.atc_gmonly && !pc_isGM(sd))
 		return;
 	if (pc_isGM(sd) < (lv=get_atcommand_level(AtCommand_Broadcast)))
 		return;
 
-	size = RFIFOW(fd,2)-4;
-	mes = (char*)RFIFOP(fd,4);
-	mes_len_check(mes, size, CHAT_SIZE);
+	text = (char*)RFIFOP(fd,4);
+	textlen = RFIFOW(fd,2) - 4;
+
+	name = text;
+	namelen = strnlen(sd->status.name, NAME_LENGTH - 1);
+	// verify <name> part of the packet
+	if( strncmp(name, sd->status.name, namelen) || // the text must start with the speaker's name
+		name[namelen] != ':' || name[namelen+1] != ' ' ) // followed by ': '
+		return;
+
+	// make sure the <message> part of the packet is safe to handle
+	message = text + namelen + 2;
+	messagelen = textlen - namelen - 2; // this should be the message length (w/ zero byte included)
+	mes_len_check(message, messagelen, CHATBOX_SIZE);
+
+	intif_GMmessage(text, textlen, 0);
 
-	intif_GMmessage(mes, size, 0);
 	if(log_config.gm && lv >= log_config.gm) {
-		char message[CHAT_SIZE+4];
-		sprintf(message, "/b %s", mes);
-		log_atcommand(sd, message);
+		char msg[CHATBOX_SIZE+4];
+		sprintf(msg, "/b %s", message);
+		log_atcommand(sd, msg);
 	}
 }
 
@@ -9468,8 +9471,7 @@ void clif_parse_UseSkillToPosSub(int fd, struct map_session_data *sd, int skilll
 			return;
 		}
 		//You can't use Graffiti/TalkieBox AND have a vending open, so this is safe.
-		memcpy(sd->message, RFIFOP(fd,skillmoreinfo), MESSAGE_SIZE);
-		sd->message[MESSAGE_SIZE-1] = '\0'; //Overflow protection [Skotlex]
+		safestrncpy(sd->message, (char*)RFIFOP(fd,skillmoreinfo), MESSAGE_SIZE);
 	}
 
 	if (sd->ud.skilltimer != -1)
@@ -9649,34 +9651,30 @@ void clif_parse_NpcNextClicked(int fd,struct map_session_data *sd)
 }
 
 /*==========================================
- *
+ * Value entered into a NPC input box.
+ * S 0143 <npcID>.l <amount>.l
  *------------------------------------------*/
 void clif_parse_NpcAmountInput(int fd,struct map_session_data *sd)
 {
-	sd->npc_amount=(int)RFIFOL(fd,6);
-	npc_scriptcont(sd,RFIFOL(fd,2));
+	int npcid = RFIFOL(fd,2);
+	int amount = (int)RFIFOL(fd,6);
+
+	sd->npc_amount = amount;
+	npc_scriptcont(sd, npcid);
 }
 
 /*==========================================
- *
+ * Text entered into a NPC input box.
+ * S 01d5 <len>.w <npcID>.l <input>.?B 00
  *------------------------------------------*/
-void clif_parse_NpcStringInput(int fd,struct map_session_data *sd)
+void clif_parse_NpcStringInput(int fd, struct map_session_data* sd)
 {
-	short message_len;
-	message_len = RFIFOW(fd,2)-7;
-
-	if(message_len < 1)
-		return; //Blank message?
-
-	if(message_len >= sizeof(sd->npc_str)){
-		ShowWarning("clif: input string too long !\n");
-		message_len = sizeof(sd->npc_str);
-	}
+	int message_len = RFIFOW(fd,2)-8;
+	int npcid = RFIFOL(fd,4);
+	const char* message = (char*)RFIFOP(fd,8);
 
-	// Exploit prevention if crafted packets (without null) is being sent. [Lance]
-	memcpy(sd->npc_str,RFIFOP(fd,8),message_len); 
-	sd->npc_str[message_len-1]=0;
-	npc_scriptcont(sd,RFIFOL(fd,4));
+	safestrncpy(sd->npc_str, message, min(message_len,CHATBOX_SIZE));
+	npc_scriptcont(sd, npcid);
 }
 
 /*==========================================
@@ -9775,31 +9773,46 @@ void clif_parse_ResetChar(int fd, struct map_session_data *sd)
 }
 
 /*==========================================
- * 019c /lb“™
+ * /lb
+ * S 019c <packet len>.w <text>.?B (<name>: <message>) 00
  *------------------------------------------*/
-void clif_parse_LGMmessage(int fd, struct map_session_data *sd)
+void clif_parse_LGMmessage(int fd, struct map_session_data* sd)
 {
-	unsigned char buf[CHAT_SIZE+4];
-	char *mes;
-	int len, lv;
+	char *text, *name, *message;
+	unsigned int textlen, namelen, messagelen;
+
+	unsigned char buf[CHATBOX_SIZE+4];
+	int lv;
 
 	if (battle_config.atc_gmonly && !pc_isGM(sd))
 		return;
 	if (pc_isGM(sd) < (lv=get_atcommand_level(AtCommand_LocalBroadcast)))
 		return;
 
-	len = RFIFOW(fd,2) - 4;
-	mes = (char*)RFIFOP(fd,4);
-	mes_len_check(mes, len, CHAT_SIZE);
+	text = (char*)RFIFOP(fd,4);
+	textlen = RFIFOW(fd,2) - 4;
+
+	name = text;
+	namelen = strnlen(sd->status.name, NAME_LENGTH - 1);
+	// verify <name> part of the packet
+	if( strncmp(name, sd->status.name, namelen) || // the text must start with the speaker's name
+		name[namelen] != ':' || name[namelen+1] != ' ' ) // followed by ': '
+		return;
+
+	// make sure the <message> part of the packet is safe to handle
+	message = text + namelen + 2;
+	messagelen = textlen - namelen - 2; // this should be the message length (w/ zero byte included)
+	mes_len_check(message, messagelen, CHATBOX_SIZE);
 
 	WBUFW(buf,0) = 0x9a;
-	WBUFW(buf,2) = len+4;
-	memcpy(WBUFP(buf,4), mes, len);
+	WBUFW(buf,2) = textlen+4;
+	memcpy(WBUFP(buf,4), text, textlen);
 	clif_send(buf, WBUFW(buf,2), &sd->bl, ALL_SAMEMAP);
+
 	if(log_config.gm && lv >= log_config.gm) {
-		char message[CHAT_SIZE+5];
-		sprintf(message, "/lb %s", mes);
-		log_atcommand(sd, message);
+		char msg[CHATBOX_SIZE+5];
+		sprintf(msg, "/lb %s", message);
+		log_atcommand(sd, msg);
 	}
 }
 
@@ -10040,12 +10053,10 @@ void clif_parse_PartyChangeOption(int fd, struct map_session_data *sd)
  *------------------------------------------*/
 void clif_parse_PartyMessage(int fd, struct map_session_data* sd)
 {
-	char* message;
-	int len;
+	int len = RFIFOW(fd,2) - 4;
+	char* message = (char*)RFIFOP(fd,4);
 
-	len = RFIFOW(fd,2) - 4;
-	message = (char*)RFIFOP(fd,4);
-	mes_len_check(message, len, CHAT_SIZE);
+	mes_len_check(message, len, CHATBOX_SIZE);
 
 	if (is_charcommand(fd, sd, message) != CharCommand_None || is_atcommand(fd, sd, message) != AtCommand_None)
 		return;
@@ -10309,12 +10320,10 @@ void clif_parse_GuildExpulsion(int fd,struct map_session_data *sd)
  *------------------------------------------*/
 void clif_parse_GuildMessage(int fd, struct map_session_data* sd)
 {
-	char* message;
-	int len;
+	int len = RFIFOW(fd,2) - 4;
+	char* message = (char*)RFIFOP(fd,4);
 
-	len = RFIFOW(fd,2) - 4;
-	message = (char*)RFIFOP(fd,4);
-	mes_len_check(message, len, CHAT_SIZE);
+	mes_len_check(message, len, CHATBOX_SIZE);
 
 	if (is_charcommand(fd, sd, message) != CharCommand_None || is_atcommand(fd, sd, message) != AtCommand_None)
 		return;