- The define MESSAGE_SIZE was wrong! It is only used for input boxes. Therefore now it is only used for Vending, Talkie box and Graffiti

- Added new define CHAT_SIZE which holds the max length that a client can send from the chat buffer. This value is NAME_LENGTH + 3 (the ' : ') + 70 (the actual text).
- Added define msg_len_check which crops incoming client text if it's longer than CHAT_SIZE. Added cropping to all incoming messages except normal chatting which is already accounted for.
- Removed variable  talkie_mes, this is now handled by sd->message
- Cleaned up parser functions for /b /lb, gm kick, /shift, /recall
- Added crash protection to the logging functions when they receive a too long string.


git-svn-id: https://svn.code.sf.net/p/rathena/svn/trunk@9778 54d463be-8e91-2dee-dedb-b68131a5f0ec

src/map/atcommand.c

@@ -1030,15 +1030,15 @@ int atcommand_config_read(const char *cfgName)
  */
 int atcommand_commands(const int fd, struct map_session_data* sd, const char* command, const char* message)
 {
-	char cz_line_buff[MESSAGE_SIZE+1];
+	char cz_line_buff[CHAT_SIZE+1];
 
 	register char *lpcz_cur = cz_line_buff;
 	register unsigned int ui_slen;
 
 	int i_cur_cmd,gm_lvl = pc_isGM(sd), count = 0;
 
-	memset(cz_line_buff,' ',MESSAGE_SIZE);
+	memset(cz_line_buff,' ',CHAT_SIZE);
-	cz_line_buff[MESSAGE_SIZE] = 0;
+	cz_line_buff[CHAT_SIZE] = 0;
 
 	clif_displaymessage(fd, msg_txt(273));
 
@@ -1050,13 +1050,13 @@ int atcommand_commands(const int fd, struct map_session_data* sd, const char* co
 		count++;
 		ui_slen = (unsigned int)strlen(atcommand_info[i_cur_cmd].command);
 
-		//rember not <= bc we need null terminator
+		//remember not <= bc we need null terminator
-		if(((MESSAGE_SIZE+(int)cz_line_buff)-(int)lpcz_cur) < (int)ui_slen)
+		if(((CHAT_SIZE+(int)cz_line_buff)-(int)lpcz_cur) < (int)ui_slen)
 		{
 			clif_displaymessage(fd,(char*)cz_line_buff);
 			lpcz_cur = cz_line_buff;
-			memset(cz_line_buff,' ',MESSAGE_SIZE);
+			memset(cz_line_buff,' ',CHAT_SIZE);
-			cz_line_buff[MESSAGE_SIZE] = 0;
+			cz_line_buff[CHAT_SIZE] = 0;
 		}
 
 		memcpy(lpcz_cur,atcommand_info[i_cur_cmd].command,ui_slen);

src/map/clif.c

@@ -92,12 +92,14 @@ struct packet_db packet_db[MAX_PACKET_VER + 1][MAX_PACKET_DB];
 
 //To idenfity disguised characters.
 #define disguised(bl) (bl->type==BL_PC && ((TBL_PC*)bl)->disguise)
+
+//Guarantees that the given string does not exceeds the allowed size, as well as making sure it's null terminated. [Skotlex\]
+#define mes_len_check(mes, len) if (len > CHAT_SIZE) { mes[CHAT_SIZE-1] = '\0'; len = CHAT_SIZE; } else mes[len-1] = '\0';
 static char map_ip_str[128];
 static in_addr_t map_ip;
 static in_addr_t bind_ip = INADDR_ANY;
 static int map_port = 5121;
 int map_fd;
-char talkie_mes[MESSAGE_SIZE];
 
 //These two will be used to verify the incoming player's validity.
 //It helps identify their client packet version correctly. [Skotlex]
@@ -7798,7 +7800,7 @@ int clif_slide(struct block_list *bl, int x, int y){
 */
 int clif_disp_overhead(struct map_session_data *sd, char* mes)
 {
-	unsigned char buf[256]; //This should be more than sufficient, the theorical max is MESSAGE_SIZE+NAME_LENGTH + 8 (pads and extra inserted crap)
+	unsigned char buf[256]; //This should be more than sufficient, the theorical max is CHAT_SIZE + 8 (pads and extra inserted crap)
 	int len_mes = strlen(mes)+1; //Account for \0
 
 	if (len_mes + 8 >= 256) {
@@ -8958,7 +8960,7 @@ void clif_parse_Wis(int fd, struct map_session_data *sd) { // S 0096 <len>.w <ni
 	msg = command;	
 	msg+= sprintf(command, "%s : ", sd->status.name);
 	memcpy(msg, RFIFOP(fd, 28), len);
-	msg[len]='\0'; //Force a terminator
+	mes_len_check(msg, len);
 	if ((is_charcommand(fd, sd, command) != CharCommand_None) ||
 		(is_atcommand(fd, sd, command) != AtCommand_None)) {
 		aFree(command);
@@ -9116,17 +9118,25 @@ void clif_parse_Wis(int fd, struct map_session_data *sd) { // S 0096 <len>.w <ni
  */
 void clif_parse_GMmessage(int fd, struct map_session_data *sd) {
 // /b
-	char message[MESSAGE_SIZE];
+	char* mes;
+	int size, lv;
 	RFIFOHEAD(fd);
-	if ((battle_config.atc_gmonly == 0 || pc_isGM(sd)) &&
+	if (battle_config.atc_gmonly && !pc_isGM(sd))
-	    (pc_isGM(sd) >= get_atcommand_level(AtCommand_Broadcast))) {
+		return;
-		intif_GMmessage((char*)RFIFOP(fd,4), RFIFOW(fd,2)-4, 0);
+	if (pc_isGM(sd) < (lv=get_atcommand_level(AtCommand_Broadcast)))
-		if((log_config.gm) && (get_atcommand_level(AtCommand_Broadcast) >= log_config.gm)) {
+		return;
-			snprintf(message, RFIFOW(fd,2)-1, "/b %s", RFIFOP(fd,4));
+
+	size = WFIFOW(fd,2)-4;
+	mes = RFIFOP(fd,4);
+	mes_len_check(mes, size);
+
+	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);
 	}
 }
-}
 
 /*==========================================
  *
@@ -9786,8 +9796,9 @@ void clif_parse_UseSkillToPosSub(int fd, struct map_session_data *sd, int skilll
 			clif_skill_fail(sd, skillnum, 0, 0);
 			return;
 		}
-		memcpy(talkie_mes, RFIFOP(fd,skillmoreinfo), MESSAGE_SIZE);
+		//You can't use Graffiti/TalkieBox AND have a vending open, so this is safe.
-		talkie_mes[MESSAGE_SIZE-1] = '\0'; //Overflow protection [Skotlex]
+		memcpy(sd->message, RFIFOP(fd,skillmoreinfo), MESSAGE_SIZE);
+		sd->message[MESSAGE_SIZE-1] = '\0'; //Overflow protection [Skotlex]
 	}
 
 	if (sd->ud.skilltimer != -1)
@@ -10117,22 +10128,30 @@ void clif_parse_ResetChar(int fd, struct map_session_data *sd) {
  *------------------------------------------
  */
 void clif_parse_LGMmessage(int fd, struct map_session_data *sd) {
-	unsigned char buf[512];
+	unsigned char buf[CHAT_SIZE+4];
-	char message[MESSAGE_SIZE];
+	char *mes;
+	int len, lv;
 	RFIFOHEAD(fd);
 
-	if ((battle_config.atc_gmonly == 0 || pc_isGM(sd)) &&
+	if (battle_config.atc_gmonly && !pc_isGM(sd))
-	    (pc_isGM(sd) >= get_atcommand_level(AtCommand_LocalBroadcast))) {
+		return;
+	if (pc_isGM(sd) < (lv=get_atcommand_level(AtCommand_LocalBroadcast)))
+		return;
+
+	len = RFIFOW(fd,2) - 4;
+	mes = RFIFOP(fd,4);
+	mes_len_check(mes, len);
+
 	WBUFW(buf,0) = 0x9a;
-		WBUFW(buf,2) = RFIFOW(fd,2);
+	WBUFW(buf,2) = len+4;
-		memcpy(WBUFP(buf,4), RFIFOP(fd,4), RFIFOW(fd,2) - 4);
+	memcpy(WBUFP(buf,4), mes, len);
-		clif_send(buf, RFIFOW(fd,2), &sd->bl, ALL_SAMEMAP);
+	clif_send(buf, WBUFW(buf,2), &sd->bl, ALL_SAMEMAP);
-		if((log_config.gm) && (get_atcommand_level(AtCommand_LocalBroadcast) >= log_config.gm)) {
+	if(log_config.gm && lv >= log_config.gm) {
-			snprintf(message, RFIFOW(fd,2), "/lb %s", RFIFOP(fd,4));
+		char message[CHAT_SIZE+5];
+		sprintf(message, "/lb %s", mes);
 		log_atcommand(sd, message);
 	}
 }
-}
 
 /*==========================================
  * ƒJƒvƒ‰‘qŒÉ‚Ö“ü‚ê‚é
@@ -10349,10 +10368,16 @@ void clif_parse_PartyChangeOption(int fd, struct map_session_data *sd) {
  *------------------------------------------
  */
 void clif_parse_PartyMessage(int fd, struct map_session_data *sd) {
-	RFIFOHEAD(fd);
+	char *mes;
+	int len;
 
-	if (is_charcommand(fd, sd, (char*)RFIFOP(fd,4)) != CharCommand_None ||
+	RFIFOHEAD(fd);
-		is_atcommand(fd, sd, (char*)RFIFOP(fd,4)) != AtCommand_None)
+	len = RFIFOW(fd,2) - 4;
+	mes = RFIFOP(fd,4);
+	mes_len_check(mes, len);
+
+	if (is_charcommand(fd, sd, mes) != CharCommand_None ||
+		is_atcommand(fd, sd, mes) != AtCommand_None)
 		return;
 
 	if	(sd->sc.count && (
@@ -10368,7 +10393,7 @@ void clif_parse_PartyMessage(int fd, struct map_session_data *sd) {
 		sd->cantalk_tick = gettick() + battle_config.min_chat_delay;
 	}
 
-	party_send_message(sd, (char*)RFIFOP(fd,4), RFIFOW(fd,2)-4);
+	party_send_message(sd, mes, len);
 }
 
 /*==========================================
@@ -10620,10 +10645,16 @@ void clif_parse_GuildExpulsion(int fd,struct map_session_data *sd) {
  *------------------------------------------
  */
 void clif_parse_GuildMessage(int fd,struct map_session_data *sd) {
-	RFIFOHEAD(fd);
+	char *mes;
+	int len;
 
-	if (is_charcommand(fd, sd, (char*)RFIFOP(fd, 4)) != CharCommand_None ||
+	RFIFOHEAD(fd);
-		is_atcommand(fd, sd, (char*)RFIFOP(fd, 4)) != AtCommand_None)
+	len = RFIFOW(fd,2) - 4;
+	mes = RFIFOP(fd,4);
+	mes_len_check(mes, len);
+
+	if (is_charcommand(fd, sd, mes) != CharCommand_None ||
+		is_atcommand(fd, sd, mes) != AtCommand_None)
 		return;
 
 	if (sd->sc.count && (
@@ -10639,7 +10670,7 @@ void clif_parse_GuildMessage(int fd,struct map_session_data *sd) {
 		sd->cantalk_tick = gettick() + battle_config.min_chat_delay;
 	}
 
-	guild_send_message(sd, (char*)RFIFOP(fd,4), RFIFOW(fd,2)-4);
+	guild_send_message(sd, mes, len);
 }
 
 /*==========================================
@@ -10776,36 +10807,47 @@ void clif_parse_ChangePetName(int fd, struct map_session_data *sd) {
 // Kick (right click menu for GM "(name) force to quit")
 void clif_parse_GMKick(int fd, struct map_session_data *sd) {
 	struct block_list *target;
-	int tid;
+	int tid,lv;
-	char message[MESSAGE_SIZE];
 
 	RFIFOHEAD(fd);
-	tid = RFIFOL(fd,2);
+	if (battle_config.atc_gmonly && !pc_isGM(sd))
+		return;
 
-	if ((battle_config.atc_gmonly == 0 || pc_isGM(sd)) &&
+	if (pc_isGM(sd) >= (lv=get_atcommand_level(AtCommand_Kick)))
-	    (pc_isGM(sd) >= get_atcommand_level(AtCommand_Kick))) {
+		return;
+
+	tid = RFIFOL(fd,2);
 	target = map_id2bl(tid);
-		if (target) {
+	if (!target) {
-			if (target->type == BL_PC) {
+		clif_GM_kickack(sd, 0);
+		return;
+	}
+
+	switch (target->type) {
+	case BL_PC:
+	{
 		struct map_session_data *tsd = (struct map_session_data *)target;
-				if (pc_isGM(sd) > pc_isGM(tsd)) {
+		if (pc_isGM(sd) <= pc_isGM(tsd)) {
+			clif_GM_kickack(sd, 0);
+			return;
+		}
 		clif_GM_kick(sd, tsd, 1);
-					if((log_config.gm) && (get_atcommand_level(AtCommand_Kick) >= log_config.gm)) {
+		if(log_config.gm && lv >= log_config.gm) {
+			char message[NAME_LENGTH+6];
 			sprintf(message, "/kick %d", tsd->status.char_id);
 			log_atcommand(sd, message);
 		}
-				} else
+		break;
-					clif_GM_kickack(sd, 0);
+	}
-			} else if (target->type == BL_MOB) {
+	case BL_MOB:
 		status_percent_damage(&sd->bl, target, 100, 0);
-				sprintf(message, "/kick %d %d %d %d", tid, target->id, target->type, target->subtype);
+		if(log_config.gm && lv >= log_config.gm) {
-				if((log_config.gm) && (get_atcommand_level(AtCommand_Kick) >= log_config.gm)) {
+			char message[NAME_LENGTH+16];
-					sprintf(message, "/kick %s", ((struct mob_data*)target)->db->sprite);
+			sprintf(message, "/kick %s (%d)", status_get_name(target), status_get_class(target));
 			log_atcommand(sd, message);
 		}
-			} else
+		break;
-				clif_GM_kickack(sd, 0);
+	default:
-		} else
 		clif_GM_kickack(sd, 0);
 	}
 }
@@ -10815,22 +10857,23 @@ void clif_parse_GMKick(int fd, struct map_session_data *sd) {
  *------------------------------------------
  */
 void clif_parse_Shift(int fd, struct map_session_data *sd) {	// Rewriten by [Yor]
-	char player_name[NAME_LENGTH];
+	char *player_name;
-	char message[MESSAGE_SIZE];
+	int lv;
 
-	memset(player_name, '\0', sizeof(player_name));
+	if (battle_config.atc_gmonly && !pc_isGM(sd))
+		return;
+	if (pc_isGM(sd) < (lv=get_atcommand_level(AtCommand_JumpTo)))
+		return;
 
-	if ((battle_config.atc_gmonly == 0 || pc_isGM(sd)) &&
-	    (pc_isGM(sd) >= get_atcommand_level(AtCommand_JumpTo))) {
 	RFIFOHEAD(fd);
-		memcpy(player_name, RFIFOP(fd,2), NAME_LENGTH);
+	player_name = RFIFOP(fd,2);
+	player_name[NAME_LENGTH-1] = '\0';
 	atcommand_jumpto(fd, sd, "@jumpto", player_name); // as @jumpto
-		if((log_config.gm) && (get_atcommand_level(AtCommand_JumpTo) >= log_config.gm)) {
+	if(log_config.gm && lv >= log_config.gm) {
+		char message[NAME_LENGTH+7];
 		sprintf(message, "/shift %s", player_name);
 		log_atcommand(sd, message);
 	}
-	}
-
 	return;
 }
 
@@ -10839,22 +10882,24 @@ void clif_parse_Shift(int fd, struct map_session_data *sd) {	// Rewriten by [Yor
  *------------------------------------------
  */
 void clif_parse_Recall(int fd, struct map_session_data *sd) {	// Added by RoVeRT
-	char player_name[NAME_LENGTH];
+	char *player_name;
-	char message[MESSAGE_SIZE];
+	int lv;
 
-	memset(player_name, '\0', sizeof(player_name));
+	if (battle_config.atc_gmonly && !pc_isGM(sd))
+		return;
+
+	if (pc_isGM(sd) < (lv=get_atcommand_level(AtCommand_Recall)))
+		return;
 
-	if ((battle_config.atc_gmonly == 0 || pc_isGM(sd)) &&
-	    (pc_isGM(sd) >= get_atcommand_level(AtCommand_Recall))) {
 	RFIFOHEAD(fd);
-		memcpy(player_name, RFIFOP(fd,2), NAME_LENGTH);
+	player_name = RFIFOP(fd,2);
+	player_name[NAME_LENGTH-1] = '\0';
 	atcommand_recall(fd, sd, "@recall", player_name); // as @recall
-		if((log_config.gm) && (get_atcommand_level(AtCommand_Recall) >= log_config.gm)) {
+	if(log_config.gm && lv >= log_config.gm) {
+		char message[NAME_LENGTH+8];
 		sprintf(message, "/recall %s", player_name);
 		log_atcommand(sd, message);
 	}
-	}
-
 	return;
 }
 
@@ -10933,7 +10978,7 @@ void clif_parse_GMReqNoChat(int fd,struct map_session_data *sd)
 	bl = map_id2bl(RFIFOL(fd,2));
 	if (!bl || bl->type != BL_PC)
 		return;
-	nullpo_retv(dstsd =(struct map_session_data *)bl);
+	dstsd =(struct map_session_data *)bl;
 
 	type = RFIFOB(fd,6);
 	limit = RFIFOW(fd,7);

src/map/log.c

@@ -12,6 +12,7 @@
 #include "itemdb.h"
 #include "map.h"
 #include "log.h"
+#include "battle.h"
 
 #ifndef SQL_DEBUG
 	#define mysql_query(_x, _y) mysql_real_query(_x, _y, strlen(_y)) //supports ' in names and runs faster [Kevin]
@@ -273,7 +274,7 @@ int log_atcommand(struct map_session_data *sd, const char *message)
 	FILE *logfp;
 #ifndef TXT_ONLY
 	char t_name[NAME_LENGTH*2];
-	char t_msg[MESSAGE_SIZE*2+1]; //These are the contents of an @ call, so there shouldn't be overflow danger here?
+	char t_msg[CHAT_SIZE*2+1]; //These are the contents of an @ call, so there shouldn't be overflow danger here?
 #endif
 
 	if(!log_config.enable_logs)
@@ -282,6 +283,12 @@ int log_atcommand(struct map_session_data *sd, const char *message)
 #ifndef TXT_ONLY
 	if(log_config.sql_logs > 0)
 	{
+		if (strlen(message) > CHAT_SIZE) {
+			if (battle_config.error_log)
+				ShowError("log atcommand: Received message too long from player %s (%d:%d)!\n",
+					sd->status.name, sd->status.account_id, sd->status.char_id);
+			return 0;
+		}
 		sprintf(tmp_sql, "INSERT DELAYED INTO `%s` (`atcommand_date`, `account_id`, `char_id`, `char_name`, `map`, `command`) VALUES(NOW(), '%d', '%d', '%s', '%s', '%s') ",
 			log_config.log_gm_db, sd->status.account_id, sd->status.char_id, jstrescapecpy(t_name, sd->status.name), mapindex_id2name(sd->mapindex), jstrescapecpy(t_msg, (char *)message));
 		if(mysql_query(&logmysql_handle, tmp_sql))
@@ -358,7 +365,7 @@ int log_chat(char *type, int type_id, int src_charid, int src_accid, char *map,
 	FILE *logfp;
 #ifndef TXT_ONLY
 	char t_charname[NAME_LENGTH*2];
-	char t_msg[MESSAGE_SIZE*2+1]; //Chat line fully escaped, with an extra space just in case.
+	char t_msg[CHAT_SIZE*2+1]; //Chat line fully escaped, with an extra space just in case.
 #endif
 	
 	//Check ON/OFF
@@ -367,6 +374,12 @@ int log_chat(char *type, int type_id, int src_charid, int src_accid, char *map,
 
 #ifndef TXT_ONLY
 	if(log_config.sql_logs > 0){
+		if (strlen(message) > CHAT_SIZE) {
+			if (battle_config.error_log)
+				ShowError("log chat: Received message too long from type %d (%d:%d)!\n",
+					type_id, src_accid, src_charid);
+			return 0;
+		}
 		sprintf(tmp_sql, "INSERT DELAYED INTO `%s` (`time`, `type`, `type_id`, `src_charid`, `src_accountid`, `src_map`, `src_map_x`, `src_map_y`, `dst_charname`, `message`) VALUES (NOW(), '%s', '%d', '%d', '%d', '%s', '%d', '%d', '%s', '%s')", 
 		 	log_config.log_chat_db, type, type_id, src_charid, src_accid, map, x, y, jstrescapecpy(t_charname, dst_charname), jstrescapecpy(t_msg, message));
 	

src/map/map.h

@@ -159,7 +159,10 @@ enum {
 	MAPID_BABY_SOUL_LINKER,
 };
 
-//Don't change this, as the client seems to always send/receive 80 characters as it currently is. [Skotlex]
+//Talk max size: <name> : <message of 70> [Skotlex]
+#define CHAT_SIZE	(NAME_LENGTH + 3 + 70)
+//Max size when inputting a string with those 'npc input boxes'
+//(also used for Graffiti, Talkie Box, Vending, and Chatrooms)
 #define MESSAGE_SIZE 80
 
 #define DEFAULT_AUTOSAVE_INTERVAL 5*60*1000
@@ -1311,8 +1314,6 @@ extern char help_txt[];
 extern char help2_txt[];
 extern char charhelp_txt[];
 
-extern char talkie_mes[];
-
 extern char wisp_server_name[];
 
 // ŽI‘S‘Ì<E28098>î•ñ

src/map/pc.c

@@ -4889,7 +4889,6 @@ int pc_dead(struct map_session_data *sd,struct block_list *src)
 	//Reset ticks.
 	sd->hp_loss_tick = sd->sp_loss_tick = 0;
 
-
 	pc_setglobalreg(sd,"PC_DIE_COUNTER",++sd->die_counter);
 
 	if (sd->state.event_death){

src/map/script.c

@@ -1045,7 +1045,6 @@ const char* parse_syntax(const char* p) {
 				if(p == p2) {
 					disp_error_message("parse_syntax: expect space ' '",p);
 				}
-				//TODO: This is incomplete as it doesn't takes into account const.txt entries!
 				// check whether case label is integer or not
 				v = strtol(p,&np,0);
 				if(np == p) { //Check for constants

src/map/skill.c

@@ -6757,8 +6757,10 @@ struct skill_unit_group *skill_unitsetting (struct block_list *src, int skillid,
 			ShowFatalError("skill_castend_map: out of memory !\n");
 			exit(1);
 		}
-		memcpy(group->valstr,talkie_mes,MESSAGE_SIZE-1);
+		if (sd)
-		group->valstr[MESSAGE_SIZE-1] = '\0';
+			memcpy(group->valstr,sd->message,MESSAGE_SIZE);
+		else //Eh... we have to write something here... even though mobs shouldn't use this. [Skotlex]
+			strcpy(group->valstr, "Boo!");
 	}
 
 	//Why redefine local variables when the ones of the function can be reused? [Skotlex]

src/map/vending.c

@@ -254,7 +254,8 @@ void vending_openvending(struct map_session_data *sd,int len,char *message,int f
 		}
 		sd->vender_id = sd->bl.id;
 		sd->vend_num = i;
-		memcpy(sd->message,message, MESSAGE_SIZE-1);
+		memcpy(sd->message,message, MESSAGE_SIZE);
+		sd->message[MESSAGE_SIZE-1] = '\0';
 		if (clif_openvending(sd,sd->vender_id,sd->vending) > 0){
 			pc_stop_walking(sd,1);
 			clif_showvendingboard(&sd->bl,message,0);