summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Changelog-Trunk.txt7
-rw-r--r--src/char_sql/int_guild.c2
-rw-r--r--src/char_sql/int_party.c2
-rw-r--r--src/common/mapindex.c2
-rw-r--r--src/common/strlib.c6
-rw-r--r--src/common/strlib.h3
-rw-r--r--src/map/clif.c111
-rw-r--r--src/map/log.c8
-rw-r--r--src/map/script.c2
9 files changed, 87 insertions, 56 deletions
diff --git a/Changelog-Trunk.txt b/Changelog-Trunk.txt
index 1062d1481..c8a9cd44d 100644
--- a/Changelog-Trunk.txt
+++ b/Changelog-Trunk.txt
@@ -7,6 +7,13 @@ IF YOU HAVE A WORKING AND TESTED BUGFIX PUT IT INTO STABLE AS WELL AS TRUNK.
* Added a sanity check for MAX_ZENY (doesn't compile if too big).
* Redid the buildin_query_sql function. (fixes bugreport:81). [FlavioJS]
2007/09/21
+ * Applied changes to clif_parse_globalmessage() from my WiP code
+ - clearer processing of the individual packet components
+ - proper code ordering, some more integrity checks
+ - fixes to some poorly chosen ShowWarning() format strings
+ - global chat logging no longer logs the entire string (w/ player name)
+ * Fixed global chat logging always crashing on a null pointer
+ * Added 'safestrnlen' to prevent null pointer crashes [ultramage]
* itemdb.c/h using a static array of 32k struct item_data* entries (faster
itemdb loockup and a first step to remove map_session_data->inventory_data).
* Fixed a typo in the configure script that replaced CFLAGS with CPPFLAGS
diff --git a/src/char_sql/int_guild.c b/src/char_sql/int_guild.c
index f33f58097..663cb4b6b 100644
--- a/src/char_sql/int_guild.c
+++ b/src/char_sql/int_guild.c
@@ -829,7 +829,7 @@ int search_guildname(char *str)
int guild_id;
char esc_name[NAME_LENGTH*2+1];
- Sql_EscapeStringLen(sql_handle, esc_name, str, strnlen(str, NAME_LENGTH));
+ Sql_EscapeStringLen(sql_handle, esc_name, str, safestrnlen(str, NAME_LENGTH));
//Lookup guilds with the same name
if( SQL_ERROR == Sql_Query(sql_handle, "SELECT guild_id FROM `%s` WHERE name='%s'", guild_db, esc_name) )
{
diff --git a/src/char_sql/int_party.c b/src/char_sql/int_party.c
index f6d783b3b..fd1f0c37c 100644
--- a/src/char_sql/int_party.c
+++ b/src/char_sql/int_party.c
@@ -299,7 +299,7 @@ struct party_data* search_partyname(char* str)
char* data;
struct party_data* p = NULL;
- Sql_EscapeStringLen(sql_handle, esc_name, str, strnlen(str, NAME_LENGTH));
+ Sql_EscapeStringLen(sql_handle, esc_name, str, safestrnlen(str, NAME_LENGTH));
if( SQL_ERROR == Sql_Query(sql_handle, "SELECT `party_id` FROM `%s` WHERE `name`='%s'", party_db, esc_name) )
Sql_ShowDebug(sql_handle);
else if( SQL_SUCCESS == Sql_NextRow(sql_handle) )
diff --git a/src/common/mapindex.c b/src/common/mapindex.c
index f2adc833c..2fa8c9fb8 100644
--- a/src/common/mapindex.c
+++ b/src/common/mapindex.c
@@ -52,7 +52,7 @@ const char* mapindex_getmapname_ext(const char* string, char* output)
static char buf[MAP_NAME_LENGTH_EXT];
char* dest = (output != NULL) ? output : buf;
- size_t len = strnlen(string, MAP_NAME_LENGTH);
+ size_t len = safestrnlen(string, MAP_NAME_LENGTH);
if (len == MAP_NAME_LENGTH) {
ShowWarning("(mapindex_normalize_name) Map name '%*s' is too long!", 2*MAP_NAME_LENGTH, string);
len--;
diff --git a/src/common/strlib.c b/src/common/strlib.c
index 4f204a768..c0a6238f9 100644
--- a/src/common/strlib.c
+++ b/src/common/strlib.c
@@ -310,6 +310,12 @@ char* safestrncpy(char* dst, const char* src, size_t n)
return ret;
}
+/// doesn't crash on null pointer
+size_t safestrnlen(const char* string, size_t maxlen)
+{
+ return ( string != NULL ) ? strnlen(string, maxlen) : 0;
+}
+
/////////////////////////////////////////////////////////////////////
// StringBuf - dynamic string
diff --git a/src/common/strlib.h b/src/common/strlib.h
index ee473f466..fd24b4b24 100644
--- a/src/common/strlib.h
+++ b/src/common/strlib.h
@@ -34,6 +34,9 @@ int config_switch(const char* str);
/// always nul-terminates the string
char* safestrncpy(char* dst, const char* src, size_t n);
+/// doesn't crash on null pointer
+size_t safestrnlen(const char* string, size_t maxlen);
+
/// StringBuf - dynamic string
struct StringBuf
{
diff --git a/src/map/clif.c b/src/map/clif.c
index fab1e00ad..5c3b62f23 100644
--- a/src/map/clif.c
+++ b/src/map/clif.c
@@ -8384,66 +8384,80 @@ void clif_parse_GetCharNameRequest(int fd, struct map_session_data *sd)
/*==========================================
* Validates and processes global messages
- * S 008c/00f3 <packet len>.w <strz>.?B
+ * S 008c/00f3 <packet len>.w <text>.?B (<name> : <message>)
*------------------------------------------*/
void clif_parse_GlobalMessage(int fd, struct map_session_data* sd)
{
- char* message;
- unsigned int packetlen, messagelen, namelen;
+ const char *text, *name, *message;
+ unsigned int packetlen, textlen, namelen, messagelen;
packetlen = RFIFOW(fd,2);
- if (packetlen > RFIFOREST(fd)) { // there has to be enough data to read
+ // 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);
return;
}
- if (packetlen < 4 + 1) { // 4-byte header and at least an empty string is expected
+ 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);
return;
}
- message = (char*)RFIFOP(fd,4);
- messagelen = packetlen - 4; // let's trust the client here, nothing can go wrong and it saves us one strlen()
- if (messagelen > CHAT_SIZE) { // messages mustn't be too long
+ text = (char*)RFIFOP(fd,4);
+ textlen = packetlen - 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] != ':' || 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);
+ 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
+ // verify <message> part of the packet
+ if( message[messagelen] != '\0' )
+ { // message must be zero-terminated
+ ShowWarning("clif_parse_GlobalMessage: Player '%s' sent an unterminated string!", sd->status.name);
+ return;
+ }
+ 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);
+ return;
+ }
+ if( messagelen > CHATBOX_SIZE )
+ { // 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]
- for(i = 0; i < MAX_SKILLTIMERSKILL && sd->ud.skilltimerskill[i] &&
- sd->ud.skilltimerskill[i]->skill_id != BA_FROSTJOKE &&
- sd->ud.skilltimerskill[i]->skill_id != DC_SCREAM; i++);
+ 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, CHAT_SIZE, message);
+ 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
+ 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;
}
}
- if (message[messagelen-1] != '\0') { // message must be zero-terminated
- ShowWarning("clif_parse_GlobalMessage: Player '%s' sent an unterminated string!", sd->status.name);
- return;
- }
- namelen = strnlen(sd->status.name, NAME_LENGTH - 1);
- if (strncmp(message, sd->status.name, namelen) || // the name has to match the speaker's name
- message[namelen] != ' ' || message[namelen+1] != ':' || message[namelen+2] != ' ') // completely, not just the prefix
- {
- //Hacked message, or infamous "client desynch" issue where they pick one char while loading another.
- clif_setwaitclose(fd); // Just kick them out to correct it.
- ShowWarning("clif_parse_GlobalMessage: Player '%.*s' sent a message using an incorrect name ('%s')! Forcing a relog...", namelen, sd->status.name, message);
- return;
- }
-
- if (is_atcommand(fd, sd, message) != AtCommand_None || is_charcommand(fd, sd, message) != CharCommand_None)
+ if( is_atcommand(fd, sd, text) != AtCommand_None || is_charcommand(fd, sd, text) != CharCommand_None )
return;
- if (sd->sc.count &&
- (sd->sc.data[SC_BERSERK].timer != -1 ||
- (sd->sc.data[SC_NOCHAT].timer != -1 && sd->sc.data[SC_NOCHAT].val1&MANNER_NOCHAT)))
+ if( sd->sc.data[SC_BERSERK].timer != -1 || (sd->sc.data[SC_NOCHAT].timer != -1 && sd->sc.data[SC_NOCHAT].val1&MANNER_NOCHAT) )
return;
- if (battle_config.min_chat_delay)
+ if( battle_config.min_chat_delay )
{ //[Skotlex]
if (DIFF_TICK(sd->cantalk_tick, gettick()) > 0)
return;
@@ -8451,11 +8465,11 @@ void clif_parse_GlobalMessage(int fd, struct map_session_data* sd)
}
// send message to others
- WFIFOHEAD(fd, messagelen + 8);
+ WFIFOHEAD(fd, 8 + textlen);
WFIFOW(fd,0) = 0x8d;
- WFIFOW(fd,2) = messagelen + 8;
+ WFIFOW(fd,2) = 8 + textlen;
WFIFOL(fd,4) = sd->bl.id;
- memcpy(WFIFOP(fd,8), message, messagelen);
+ safestrncpy((char*)WFIFOP(fd,8), text, textlen);
clif_send(WFIFOP(fd,0), WFIFOW(fd,2), &sd->bl, sd->chatID ? CHAT_WOS : AREA_CHAT_WOC);
// send back message to the speaker
@@ -8465,37 +8479,38 @@ void clif_parse_GlobalMessage(int fd, struct map_session_data* sd)
#ifdef PCRE_SUPPORT
// trigger listening mobs/npcs
- map_foreachinrange(npc_chat_sub, &sd->bl, AREA_SIZE, BL_NPC, message, strlen(message), &sd->bl);
- map_foreachinrange(mob_chat_sub, &sd->bl, AREA_SIZE, BL_MOB, message, strlen(message), &sd->bl);
+ map_foreachinrange(npc_chat_sub, &sd->bl, AREA_SIZE, BL_NPC, text, textlen, &sd->bl);
+ map_foreachinrange(mob_chat_sub, &sd->bl, AREA_SIZE, BL_MOB, text, textlen, &sd->bl);
#endif
// check for special supernovice phrase
- if ((sd->class_&MAPID_UPPERMASK) == MAPID_SUPER_NOVICE) {
+ if( (sd->class_&MAPID_UPPERMASK) == MAPID_SUPER_NOVICE )
+ {
int next = pc_nextbaseexp(sd);
- if (next > 0 && (sd->status.base_exp * 1000 / next)% 100 == 0) { // 0%, 10%, 20%, ...
+ if( next > 0 && (sd->status.base_exp * 1000 / next)% 100 == 0 ) { // 0%, 10%, 20%, ...
switch (sd->state.snovice_call_flag) {
case 0:
- if (strstr(message, msg_txt(504))) // "Guardian Angel, can you hear my voice? ^^;"
+ if( strstr(message, msg_txt(504)) ) // "Guardian Angel, can you hear my voice? ^^;"
sd->state.snovice_call_flag++;
break;
case 1: {
char buf[256];
sprintf(buf, msg_txt(505), sd->status.name);
- if (strstr(message, buf)) // "My name is %s, and I'm a Super Novice~"
+ if( strstr(message, buf) ) // "My name is %s, and I'm a Super Novice~"
sd->state.snovice_call_flag++;
}
break;
case 2:
- if (strstr(message, msg_txt(506))) // "Please help me~ T.T"
+ if( strstr(message, msg_txt(506)) ) // "Please help me~ T.T"
sd->state.snovice_call_flag++;
break;
case 3:
- if (skillnotok(MO_EXPLOSIONSPIRITS,sd))
- break; //Do not override the noskill mapflag. [Skotlex]
- clif_skill_nodamage(&sd->bl,&sd->bl,MO_EXPLOSIONSPIRITS,-1,
- sc_start(&sd->bl,SkillStatusChangeTable(MO_EXPLOSIONSPIRITS),100,
+ if( skillnotok(MO_EXPLOSIONSPIRITS,sd) )
+ break; //Do not override the noskill mapflag. [Skotlex]
+ clif_skill_nodamage(&sd->bl,&sd->bl,MO_EXPLOSIONSPIRITS,-1,
+ sc_start(&sd->bl,SkillStatusChangeTable(MO_EXPLOSIONSPIRITS),100,
17,skill_get_time(MO_EXPLOSIONSPIRITS,1))); //Lv17-> +50 critical (noted by Poki) [Skotlex]
- sd->state.snovice_call_flag = 0;
+ sd->state.snovice_call_flag = 0;
break;
}
}
diff --git a/src/map/log.c b/src/map/log.c
index b5c4a3d9c..40225e7b7 100644
--- a/src/map/log.c
+++ b/src/map/log.c
@@ -298,7 +298,7 @@ int log_atcommand(struct map_session_data* sd, const char* message)
stmt = SqlStmt_Malloc(logmysql_handle);
if( SQL_SUCCESS != SqlStmt_Prepare(stmt, "INSERT DELAYED INTO `%s` (`atcommand_date`, `account_id`, `char_id`, `char_name`, `map`, `command`) VALUES (NOW(), '%d', '%d', ?, '%s', ?)", log_config.log_gm_db, sd->status.account_id, sd->status.char_id, sd->status.name, mapindex_id2name(sd->mapindex), message)
|| SQL_SUCCESS != SqlStmt_BindParam(stmt, 0, SQLDT_STRING, sd->status.name, strnlen(sd->status.name, NAME_LENGTH))
- || SQL_SUCCESS != SqlStmt_BindParam(stmt, 1, SQLDT_STRING, (char*)message, strnlen(message, 255))
+ || SQL_SUCCESS != SqlStmt_BindParam(stmt, 1, SQLDT_STRING, (char*)message, safestrnlen(message, 255))
|| SQL_SUCCESS != SqlStmt_Execute(stmt) )
{
SqlStmt_ShowDebug(stmt);
@@ -336,7 +336,7 @@ int log_npc(struct map_session_data* sd, const char* message)
stmt = SqlStmt_Malloc(logmysql_handle);
if( SQL_SUCCESS != SqlStmt_Prepare(stmt, "INSERT DELAYED INTO `%s` (`npc_date`, `account_id`, `char_id`, `char_name`, `map`, `mes`) VALUES (NOW(), '%d', '%d', ?, '%s', ?)", log_config.log_npc_db, sd->status.account_id, sd->status.char_id, sd->status.name, mapindex_id2name(sd->mapindex), message)
|| SQL_SUCCESS != SqlStmt_BindParam(stmt, 0, SQLDT_STRING, sd->status.name, strnlen(sd->status.name, NAME_LENGTH))
- || SQL_SUCCESS != SqlStmt_BindParam(stmt, 1, SQLDT_STRING, (char*)message, strnlen(message, 255))
+ || SQL_SUCCESS != SqlStmt_BindParam(stmt, 1, SQLDT_STRING, (char*)message, safestrnlen(message, 255))
|| SQL_SUCCESS != SqlStmt_Execute(stmt) )
{
SqlStmt_ShowDebug(stmt);
@@ -393,8 +393,8 @@ int log_chat(const char* type, int type_id, int src_charid, int src_accid, const
stmt = SqlStmt_Malloc(logmysql_handle);
if( SQL_SUCCESS != SqlStmt_Prepare(stmt, "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', ?, ?)", log_config.log_chat_db, type, type_id, src_charid, src_accid, map, x, y)
- || SQL_SUCCESS != SqlStmt_BindParam(stmt, 0, SQLDT_STRING, (char*)dst_charname, strnlen(dst_charname, NAME_LENGTH))
- || SQL_SUCCESS != SqlStmt_BindParam(stmt, 1, SQLDT_STRING, (char*)message, strnlen(message, CHAT_SIZE))
+ || SQL_SUCCESS != SqlStmt_BindParam(stmt, 0, SQLDT_STRING, (char*)dst_charname, safestrnlen(dst_charname, NAME_LENGTH))
+ || SQL_SUCCESS != SqlStmt_BindParam(stmt, 1, SQLDT_STRING, (char*)message, safestrnlen(message, CHAT_SIZE))
|| SQL_SUCCESS != SqlStmt_Execute(stmt) )
{
SqlStmt_ShowDebug(stmt);
diff --git a/src/map/script.c b/src/map/script.c
index 5dff55cac..37dd99229 100644
--- a/src/map/script.c
+++ b/src/map/script.c
@@ -3423,7 +3423,7 @@ static int script_save_mapreg_strsub(DBKey key,void *data,va_list ap)
#else
if ( name[1] != '@') {
char tmp_str2[2*255+1];
- Sql_EscapeStringLen(mmysql_handle, tmp_str2, (char*)data, strnlen((char*)data, 255));
+ Sql_EscapeStringLen(mmysql_handle, tmp_str2, (char*)data, safestrnlen((char*)data, 255));
if( SQL_ERROR == Sql_Query(mmysql_handle, "UPDATE `%s` SET `%s`='%s' WHERE `%s`='%s' AND `%s`='%d'", mapregsql_db, mapregsql_db_value, tmp_str2, mapregsql_db_varname, name, mapregsql_db_index, i) )
Sql_ShowDebug(mmysql_handle);
}
generated by cgit v1.2.3-70-g09d2 (git 2.46.0) at 2024-11-04 21:15:37 +0000