From 05aa1ffc8c5b4cbe21c3de6fe406c317ab557a9f Mon Sep 17 00:00:00 2001 From: lighta Date: Thu, 29 Oct 2015 23:43:48 -0400 Subject: [PATCH] Type enforcement Change the type of father_id,mother_id,child_id mail.zeny,mail.send_id,mail.dest_id from int to uint32, as this is their DB representation anyway and most of those are mapped to char_id which is uint32. This should remove the need of implicite cast and improve security overall. --- src/char/char.c | 10 +++++----- src/char/int_mail.c | 8 ++++---- src/common/mmo.h | 16 ++++++++-------- src/map/atcommand.c | 4 ++-- src/map/chrif.c | 2 +- src/map/clif.c | 26 ++++++++++++-------------- src/map/mail.c | 8 ++++---- src/map/mail.h | 2 +- src/map/map.c | 8 ++++---- 9 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/char/char.c b/src/char/char.c index c59baf537b..b86045f347 100644 --- a/src/char/char.c +++ b/src/char/char.c @@ -373,7 +373,7 @@ int char_mmo_char_tosql(uint32 char_id, struct mmo_charstatus* p){ { if( SQL_ERROR == Sql_Query(sql_handle, "UPDATE `%s` SET `class`='%d'," "`hair`='%d',`hair_color`='%d',`clothes_color`='%d'," - "`partner_id`='%d', `father`='%d', `mother`='%d', `child`='%d'," + "`partner_id`='%u', `father`='%u', `mother`='%u', `child`='%u'," "`karma`='%d',`manner`='%d', `fame`='%d'" " WHERE `account_id`='%d' AND `char_id` = '%d'", schema_config.char_db, p->class_, @@ -1080,10 +1080,10 @@ int char_mmo_char_fromsql(uint32 char_id, struct mmo_charstatus* p, bool load_ev || SQL_ERROR == SqlStmt_BindColumn(stmt, 41, SQLDT_STRING, &save_map, sizeof(save_map), NULL, NULL) || SQL_ERROR == SqlStmt_BindColumn(stmt, 42, SQLDT_SHORT, &p->save_point.x, 0, NULL, NULL) || SQL_ERROR == SqlStmt_BindColumn(stmt, 43, SQLDT_SHORT, &p->save_point.y, 0, NULL, NULL) - || SQL_ERROR == SqlStmt_BindColumn(stmt, 44, SQLDT_INT, &p->partner_id, 0, NULL, NULL) - || SQL_ERROR == SqlStmt_BindColumn(stmt, 45, SQLDT_INT, &p->father, 0, NULL, NULL) - || SQL_ERROR == SqlStmt_BindColumn(stmt, 46, SQLDT_INT, &p->mother, 0, NULL, NULL) - || SQL_ERROR == SqlStmt_BindColumn(stmt, 47, SQLDT_INT, &p->child, 0, NULL, NULL) + || SQL_ERROR == SqlStmt_BindColumn(stmt, 44, SQLDT_UINT32, &p->partner_id, 0, NULL, NULL) + || SQL_ERROR == SqlStmt_BindColumn(stmt, 45, SQLDT_UINT32, &p->father, 0, NULL, NULL) + || SQL_ERROR == SqlStmt_BindColumn(stmt, 46, SQLDT_UINT32, &p->mother, 0, NULL, NULL) + || SQL_ERROR == SqlStmt_BindColumn(stmt, 47, SQLDT_UINT32, &p->child, 0, NULL, NULL) || SQL_ERROR == SqlStmt_BindColumn(stmt, 48, SQLDT_INT, &p->fame, 0, NULL, NULL) || SQL_ERROR == SqlStmt_BindColumn(stmt, 49, SQLDT_SHORT, &p->rename, 0, NULL, NULL) || SQL_ERROR == SqlStmt_BindColumn(stmt, 50, SQLDT_UINT32, &p->delete_date, 0, NULL, NULL) diff --git a/src/char/int_mail.c b/src/char/int_mail.c index 55f6c8f4b7..c91d9c0c61 100644 --- a/src/char/int_mail.c +++ b/src/char/int_mail.c @@ -45,14 +45,14 @@ static int mail_fromsql(uint32 char_id, struct mail_data* md) msg = &md->msg[i]; Sql_GetData(sql_handle, 0, &data, NULL); msg->id = atoi(data); Sql_GetData(sql_handle, 1, &data, NULL); safestrncpy(msg->send_name, data, NAME_LENGTH); - Sql_GetData(sql_handle, 2, &data, NULL); msg->send_id = atoi(data); + Sql_GetData(sql_handle, 2, &data, NULL); msg->send_id = strtoul(data, NULL, 10); Sql_GetData(sql_handle, 3, &data, NULL); safestrncpy(msg->dest_name, data, NAME_LENGTH); - Sql_GetData(sql_handle, 4, &data, NULL); msg->dest_id = atoi(data); + Sql_GetData(sql_handle, 4, &data, NULL); msg->dest_id = strtoul(data, NULL, 10); Sql_GetData(sql_handle, 5, &data, NULL); safestrncpy(msg->title, data, MAIL_TITLE_LENGTH); Sql_GetData(sql_handle, 6, &data, NULL); safestrncpy(msg->body, data, MAIL_BODY_LENGTH); - Sql_GetData(sql_handle, 7, &data, NULL); msg->timestamp = atoi(data); + Sql_GetData(sql_handle, 7, &data, NULL); msg->timestamp = atoi(data); //strtoull ? Sql_GetData(sql_handle, 8, &data, NULL); msg->status = (mail_status)atoi(data); - Sql_GetData(sql_handle, 9, &data, NULL); msg->zeny = atoi(data); + Sql_GetData(sql_handle, 9, &data, NULL); msg->zeny = strtoul(data, NULL, 10); item = &msg->item; Sql_GetData(sql_handle,10, &data, NULL); item->amount = (short)atoi(data); Sql_GetData(sql_handle,11, &data, NULL); item->nameid = atoi(data); diff --git a/src/common/mmo.h b/src/common/mmo.h index dd5b82ab90..a540d2d92c 100644 --- a/src/common/mmo.h +++ b/src/common/mmo.h @@ -374,9 +374,9 @@ struct mmo_charstatus { uint32 char_id; uint32 account_id; uint32 partner_id; - int father; - int mother; - int child; + uint32 father; + uint32 mother; + uint32 child; unsigned int base_exp,job_exp; int zeny; @@ -444,17 +444,17 @@ typedef enum mail_status { struct mail_message { int id; - int send_id; - char send_name[NAME_LENGTH]; - int dest_id; - char dest_name[NAME_LENGTH]; + uint32 send_id; //hold char_id of sender + char send_name[NAME_LENGTH]; //sender nickname + uint32 dest_id; //hold char_id of receiver + char dest_name[NAME_LENGTH]; //receiver nickname char title[MAIL_TITLE_LENGTH]; char body[MAIL_BODY_LENGTH]; mail_status status; time_t timestamp; // marks when the message was sent - int zeny; + uint32 zeny; struct item item; }; diff --git a/src/map/atcommand.c b/src/map/atcommand.c index a376997f65..17866a313f 100644 --- a/src/map/atcommand.c +++ b/src/map/atcommand.c @@ -9582,7 +9582,7 @@ ACMD_FUNC(cloneequip) { nullpo_retr(-1, sd); memset(atcmd_output, '\0', sizeof(atcmd_output)); - if( !message || !*message || (sscanf(message, "%d", &char_id) < 1 && sscanf(message, "\"%23[^\"]\"", atcmd_output) < 1) ) { + if( !message || !*message || (sscanf(message, "%u", &char_id) < 1 && sscanf(message, "\"%23[^\"]\"", atcmd_output) < 1) ) { clif_displaymessage(fd, msg_txt(sd, 735)); // Please enter char_id or \"char name\". return -1; } @@ -9658,7 +9658,7 @@ ACMD_FUNC(clonestat) { nullpo_retr(-1, sd); memset(atcmd_output, '\0', sizeof(atcmd_output)); - if( !message || !*message || (sscanf(message, "%d", &char_id) < 1 && sscanf(message, "\"%23[^\"]\"", atcmd_output) < 1) ) { + if( !message || !*message || (sscanf(message, "%u", &char_id) < 1 && sscanf(message, "\"%23[^\"]\"", atcmd_output) < 1) ) { clif_displaymessage(fd, msg_txt(sd, 735)); // Please enter char_id or \"char name\". return -1; } diff --git a/src/map/chrif.c b/src/map/chrif.c index feda226a40..8e728fa7e9 100644 --- a/src/map/chrif.c +++ b/src/map/chrif.c @@ -1049,7 +1049,7 @@ int chrif_divorceack(uint32 char_id, int partner_id) { /*========================================== * Removes Baby from parents *------------------------------------------*/ -int chrif_deadopt(int father_id, int mother_id, int child_id) { +int chrif_deadopt(uint32 father_id, uint32 mother_id, uint32 child_id) { struct map_session_data* sd; uint16 idx = skill_get_index(WE_CALLBABY); diff --git a/src/map/clif.c b/src/map/clif.c index aa8f0021b7..4f5ca4e91f 100644 --- a/src/map/clif.c +++ b/src/map/clif.c @@ -11573,7 +11573,7 @@ static void clif_parse_UseSkillToPos_mercenary(struct mercenary_data *md, struct void clif_parse_UseSkillToId(int fd, struct map_session_data *sd) { uint16 skill_id, skill_lv; - int tmp, target_id; + int inf,target_id; unsigned int tick = gettick(); struct s_packet_db* info = &packet_db[sd->packet_ver][RFIFOW(fd,0)]; @@ -11583,8 +11583,8 @@ void clif_parse_UseSkillToId(int fd, struct map_session_data *sd) if( skill_lv < 1 ) skill_lv = 1; //No clue, I have seen the client do this with guild skills :/ [Skotlex] - tmp = skill_get_inf(skill_id); - if (tmp&INF_GROUND_SKILL || !tmp) + inf = skill_get_inf(skill_id); + if (inf&INF_GROUND_SKILL || !inf) return; //Using a ground/passive skill on a target? WRONG. if( SKILL_CHK_HOMUN(skill_id) ) { @@ -11606,14 +11606,14 @@ void clif_parse_UseSkillToId(int fd, struct map_session_data *sd) clif_msg(sd, USAGE_FAIL); // TODO look for the client date that has this message. return; #else - if( !sd->npc_item_flag || !(tmp&INF_SELF_SKILL) ) + if( !sd->npc_item_flag || !(inf&INF_SELF_SKILL) ) return; #endif } if( (pc_cant_act2(sd) || sd->chatID) && skill_id != RK_REFRESH && !(skill_id == SR_GENTLETOUCH_CURE && (sd->sc.opt1 == OPT1_STONE || sd->sc.opt1 == OPT1_FREEZE || sd->sc.opt1 == OPT1_STUN)) && - sd->state.storage_flag && !(tmp&INF_SELF_SKILL) ) //SELF skills can be used with the storage open, issue: 8027 + sd->state.storage_flag && !(inf&INF_SELF_SKILL) ) //SELF skills can be used with the storage open, issue: 8027 return; if( pc_issit(sd) ) @@ -11622,7 +11622,7 @@ void clif_parse_UseSkillToId(int fd, struct map_session_data *sd) if( skill_isNotOk(skill_id, sd) ) return; - if( sd->bl.id != target_id && tmp&INF_SELF_SKILL ) + if( sd->bl.id != target_id && inf&INF_SELF_SKILL ) target_id = sd->bl.id; // never trust the client if( target_id < 0 && -target_id == sd->bl.id ) // for disguises [Valaris] @@ -11650,15 +11650,15 @@ void clif_parse_UseSkillToId(int fd, struct map_session_data *sd) } else if( sd->menuskill_id != SA_AUTOSPELL ) return; //Can't use skills while a menu is open. } + if( sd->skillitem == skill_id ) { if( skill_lv != sd->skillitemlv ) skill_lv = sd->skillitemlv; - if( !(tmp&INF_SELF_SKILL) ) + if( !(inf&INF_SELF_SKILL) ) pc_delinvincibletimer(sd); // Target skills thru items cancel invincibility. [Inkfish] unit_skilluse_id(&sd->bl, target_id, skill_id, skill_lv); return; } - sd->skillitem = sd->skillitemlv = 0; if( SKILL_CHK_GUILD(skill_id) ) { @@ -11667,9 +11667,7 @@ void clif_parse_UseSkillToId(int fd, struct map_session_data *sd) else skill_lv = 0; } else { - tmp = pc_checkskill(sd, skill_id); - if( skill_lv > tmp ) - skill_lv = tmp; + skill_lv = min(pc_checkskill(sd, skill_id),skill_lv); //never trust client } pc_delinvincibletimer(sd); @@ -14511,11 +14509,11 @@ void clif_Mail_setattachment(int fd, int index, uint8 flag) /// 0 = success /// 1 = failure /// 2 = too many items -void clif_Mail_getattachment(int fd, uint8 flag) +void clif_Mail_getattachment(int fd, uint8 result) { WFIFOHEAD(fd,packet_len(0x245)); WFIFOW(fd,0) = 0x245; - WFIFOB(fd,2) = flag; + WFIFOB(fd,2) = result; WFIFOSET(fd,packet_len(0x245)); } @@ -14748,7 +14746,7 @@ void clif_parse_Mail_getattach(int fd, struct map_session_data *sd) return; if( sd->mail.inbox.msg[i].zeny + sd->status.zeny > MAX_ZENY ) { - clif_Mail_getattachment(fd, 1); + clif_Mail_getattachment(fd, 1); //too many zeny return; } diff --git a/src/map/mail.c b/src/map/mail.c index a5acb13815..e6550eb440 100644 --- a/src/map/mail.c +++ b/src/map/mail.c @@ -56,13 +56,13 @@ int mail_removezeny(struct map_session_data *sd, short flag) } /** -* Attempt to set item or zeny -* @param sd +* Attempt to set item or zeny to a mail +* @param sd : player attaching the content * @param idx 0 - Zeny; >= 2 - Inventory item -* @param amount +* @param amount : amout of zeny or number of item * @return True if item/zeny can be set, False if failed */ -bool mail_setitem(struct map_session_data *sd, short idx, int amount) { +bool mail_setitem(struct map_session_data *sd, short idx, uint32 amount) { if( pc_istrading(sd) ) return false; diff --git a/src/map/mail.h b/src/map/mail.h index d0c8e5396a..5790526112 100644 --- a/src/map/mail.h +++ b/src/map/mail.h @@ -9,7 +9,7 @@ void mail_clear(struct map_session_data *sd); int mail_removeitem(struct map_session_data *sd, short flag); int mail_removezeny(struct map_session_data *sd, short flag); -bool mail_setitem(struct map_session_data *sd, short idx, int amount); +bool mail_setitem(struct map_session_data *sd, short idx, uint32 amount); bool mail_setattachment(struct map_session_data *sd, struct mail_message *msg); void mail_getattachment(struct map_session_data* sd, int zeny, struct item* item); int mail_openmail(struct map_session_data *sd); diff --git a/src/map/map.c b/src/map/map.c index 7e331e4d7a..2c4a085205 100644 --- a/src/map/map.c +++ b/src/map/map.c @@ -1677,7 +1677,7 @@ void map_addiddb(struct block_list *bl) { TBL_PC* sd = (TBL_PC*)bl; idb_put(pc_db,sd->bl.id,sd); - idb_put(charid_db,sd->status.char_id,sd); + uidb_put(charid_db,sd->status.char_id,sd); } else if( bl->type == BL_MOB ) { @@ -1705,7 +1705,7 @@ void map_deliddb(struct block_list *bl) { TBL_PC* sd = (TBL_PC*)bl; idb_remove(pc_db,sd->bl.id); - idb_remove(charid_db,sd->status.char_id); + uidb_remove(charid_db,sd->status.char_id); } else if( bl->type == BL_MOB ) { @@ -1932,7 +1932,7 @@ const char* map_charid2nick(int charid) /// Returns the struct map_session_data of the charid or NULL if the char is not online. struct map_session_data* map_charid2sd(int charid) { - return (struct map_session_data*)idb_get(charid_db, charid); + return (struct map_session_data*)uidb_get(charid_db, charid); } /*========================================== @@ -4383,7 +4383,7 @@ int do_init(int argc, char *argv[]) bossid_db = idb_alloc(DB_OPT_BASE); // Used for Convex Mirror quick MVP search map_db = uidb_alloc(DB_OPT_BASE); nick_db = idb_alloc(DB_OPT_BASE); - charid_db = idb_alloc(DB_OPT_BASE); + charid_db = uidb_alloc(DB_OPT_BASE); regen_db = idb_alloc(DB_OPT_BASE); // efficient status_natural_heal processing iwall_db = strdb_alloc(DB_OPT_RELEASE_DATA,2*NAME_LENGTH+2+1); // [Zephyrus] Invisible Walls