From a3c4d675ba19df385be5d1e3966c61de7186da57 Mon Sep 17 00:00:00 2001 From: shennetsind Date: Sat, 17 Jan 2015 20:18:49 -0200 Subject: 27 Fixes Addressing out of bounds read/write, pointless null checks on already deferenced variables. Special Thanks to 4144 and Haruna! Signed-off-by: shennetsind --- src/char/inter.c | 36 +++++++++++++++--------------------- src/common/grfio.c | 2 +- src/map/atcommand.c | 3 ++- src/map/battle.c | 3 ++- src/map/clif.c | 16 ++++++++-------- src/map/itemdb.c | 2 +- src/map/mercenary.c | 2 +- src/map/mob.c | 2 +- src/map/npc.c | 2 +- src/map/pc.c | 33 ++++++++++++++++----------------- src/map/script.c | 20 ++++++++++++-------- src/map/skill.c | 39 +++++++++++++++++++++------------------ src/map/status.c | 51 +++++++++++++++++++++++++++------------------------ 13 files changed, 108 insertions(+), 103 deletions(-) (limited to 'src') diff --git a/src/char/inter.c b/src/char/inter.c index e60da2b4f..6cd34dc39 100644 --- a/src/char/inter.c +++ b/src/char/inter.c @@ -780,24 +780,18 @@ static int inter_config_read(const char* cfgName) continue; if(!strcmpi(w1,"char_server_ip")) { - strcpy(char_server_ip,w2); - } else - if(!strcmpi(w1,"char_server_port")) { + safestrncpy(char_server_ip, w2, sizeof(char_server_ip)); + } else if(!strcmpi(w1,"char_server_port")) { char_server_port = atoi(w2); - } else - if(!strcmpi(w1,"char_server_id")) { - strcpy(char_server_id,w2); - } else - if(!strcmpi(w1,"char_server_pw")) { - strcpy(char_server_pw,w2); - } else - if(!strcmpi(w1,"char_server_db")) { - strcpy(char_server_db,w2); - } else - if(!strcmpi(w1,"default_codepage")) { - strcpy(default_codepage,w2); - } - else if(!strcmpi(w1,"party_share_level")) + } else if(!strcmpi(w1,"char_server_id")) { + safestrncpy(char_server_id, w2, sizeof(char_server_id)); + } else if(!strcmpi(w1,"char_server_pw")) { + safestrncpy(char_server_pw, w2, sizeof(char_server_pw)); + } else if(!strcmpi(w1,"char_server_db")) { + safestrncpy(char_server_db, w2, sizeof(char_server_db)); + } else if(!strcmpi(w1,"default_codepage")) { + safestrncpy(default_codepage, w2, sizeof(default_codepage)); + } else if(!strcmpi(w1,"party_share_level")) party_share_level = atoi(w2); else if(!strcmpi(w1,"log_inter")) log_inter = atoi(w2); @@ -931,8 +925,7 @@ int mapif_broadcast(unsigned char *mes, int len, unsigned int fontColor, short f memcpy(WBUFP(buf,16), mes, len - 16); mapif->sendallwos(sfd, buf, len); - if (buf) - aFree(buf); + aFree(buf); return 0; } @@ -940,8 +933,9 @@ int mapif_broadcast(unsigned char *mes, int len, unsigned int fontColor, short f int mapif_wis_message(struct WisData *wd) { unsigned char buf[2048]; - if (wd->len > 2047-56) wd->len = 2047-56; //Force it to fit to avoid crashes. [Skotlex] - + //if (wd->len > 2047-56) wd->len = 2047-56; //Force it to fit to avoid crashes. [Skotlex] + if( wd->len >= sizeof(wd->msg) - 1 ) wd->len = sizeof(wd->msg) - 1; + WBUFW(buf, 0) = 0x3801; WBUFW(buf, 2) = 56 +wd->len; WBUFL(buf, 4) = wd->id; diff --git a/src/common/grfio.c b/src/common/grfio.c index eeda7e4b5..28e6c87f4 100644 --- a/src/common/grfio.c +++ b/src/common/grfio.c @@ -509,7 +509,7 @@ static bool isFullEncrypt(const char* fname) static int grfio_entryread(const char* grfname, int gentry) { long grf_size; - unsigned char grf_header[0x2e]; + unsigned char grf_header[0x2e] = { 0 }; int entry,entrys,ofs,grf_version; unsigned char *grf_filelist; diff --git a/src/map/atcommand.c b/src/map/atcommand.c index 56dd5784c..a947b8f47 100644 --- a/src/map/atcommand.c +++ b/src/map/atcommand.c @@ -5199,7 +5199,8 @@ ACMD(skillid) { sprintf(atcmd_output, msg_txt(1164), DB->data2i(data), skill->db[idx].desc, key.str); // skill %d: %s (%s) clif->message(fd, atcmd_output); } else if ( found < MAX_SKILLID_PARTIAL_RESULTS && ( stristr(key.str,message) || stristr(skill->db[idx].desc,message) ) ) { - snprintf(partials[found++], MAX_SKILLID_PARTIAL_RESULTS_LEN, msg_txt(1164), DB->data2i(data), skill->db[idx].desc, key.str); + snprintf(partials[found], MAX_SKILLID_PARTIAL_RESULTS_LEN, msg_txt(1164), DB->data2i(data), skill->db[idx].desc, key.str); + found++; } } diff --git a/src/map/battle.c b/src/map/battle.c index 7e7317935..f332dd330 100644 --- a/src/map/battle.c +++ b/src/map/battle.c @@ -4783,6 +4783,7 @@ struct Damage battle_calc_weapon_attack(struct block_list *src,struct block_list #ifdef RENEWAL case NJ_TATAMIGAESHI: ATK_RATE(200); + /* Fall through */ case LK_SPIRALPIERCE: case ML_SPIRALPIERCE: // [malufett] if( skill_id != NJ_TATAMIGAESHI ){ @@ -5685,7 +5686,7 @@ int battle_damage_area(struct block_list *bl, va_list ap) { else status_fix_damage(src,bl,damage,0); clif->damage(bl,bl,amotion,dmotion,damage,1,ATK_BLOCK,0); - if( !(src && src->type == BL_PC && ((TBL_PC*)src)->state.autocast) ) + if( !(src->type == BL_PC && ((TBL_PC*)src)->state.autocast) ) skill->additional_effect(src, bl, CR_REFLECTSHIELD, 1, BF_WEAPON|BF_SHORT|BF_NORMAL,ATK_DEF,tick); map->freeblock_unlock(); } diff --git a/src/map/clif.c b/src/map/clif.c index 31f7961d5..ae6b6d939 100644 --- a/src/map/clif.c +++ b/src/map/clif.c @@ -5706,8 +5706,7 @@ void clif_broadcast(struct block_list* bl, const char* mes, size_t len, int type memcpy(WBUFP(buf, 4 + lp), mes, len); clif->send(buf, WBUFW(buf,2), bl, target); - if (buf) - aFree(buf); + aFree(buf); } /*========================================== @@ -5753,8 +5752,7 @@ void clif_broadcast2(struct block_list* bl, const char* mes, size_t len, unsigne memcpy(WBUFP(buf,16), mes, len); clif->send(buf, WBUFW(buf,2), bl, target); - if (buf) - aFree(buf); + aFree(buf); } @@ -16299,6 +16297,7 @@ void clif_bg_message(struct battleground_data *bgd, int src_id, const char *name { struct map_session_data *sd; unsigned char *buf; + if( !bgd->count || (sd = bg->getavailablesd(bgd)) == NULL ) return; @@ -16311,8 +16310,7 @@ void clif_bg_message(struct battleground_data *bgd, int src_id, const char *name memcpy(WBUFP(buf,32), mes, len); clif->send(buf,WBUFW(buf,2), &sd->bl, BG); - if( buf ) - aFree(buf); + aFree(buf); } @@ -18816,10 +18814,12 @@ static void __attribute__ ((unused)) packetdb_addpacket(short cmd, int len, ...) pos = va_arg(va, int); - if( pos == 0xFFFF ) /* nothing more to do */ + va_end(va); + + if( pos == 0xFFFF ) { /* nothing more to do */ return; + } - va_end(va); va_start(va,len); func = va_arg(va,pFunc); diff --git a/src/map/itemdb.c b/src/map/itemdb.c index b537d69be..be182be72 100644 --- a/src/map/itemdb.c +++ b/src/map/itemdb.c @@ -764,7 +764,7 @@ void itemdb_write_cached_packages(const char *config_filename) { } bool itemdb_read_cached_packages(const char *config_filename) { FILE *file; - unsigned short pcount; + unsigned short pcount = 0; unsigned short i; if( !(file = HCache->open(config_filename,"rb")) ) { diff --git a/src/map/mercenary.c b/src/map/mercenary.c index a1503e97a..84f6a3c41 100644 --- a/src/map/mercenary.c +++ b/src/map/mercenary.c @@ -333,7 +333,7 @@ int merc_data_received(struct s_mercenary *merc, bool flag) { mercenary->set_calls(md, 1); sd->status.mer_id = merc->mercenary_id; - if( md && md->bl.prev == NULL && sd->bl.prev != NULL ) { + if( md->bl.prev == NULL && sd->bl.prev != NULL ) { map->addblock(&md->bl); clif->spawn(&md->bl); clif->mercenary_info(sd); diff --git a/src/map/mob.c b/src/map/mob.c index 30a359bc7..0eb1a9833 100644 --- a/src/map/mob.c +++ b/src/map/mob.c @@ -3894,7 +3894,7 @@ bool mob_parse_dbrow(char** str) { if (k == MAX_SEARCH) continue; - if (id->mob[k].id != class_) + if (id->mob[k].id != class_ && k != MAX_SEARCH - 1) memmove(&id->mob[k+1], &id->mob[k], (MAX_SEARCH-k-1)*sizeof(id->mob[0])); id->mob[k].chance = db->dropitem[i].p; id->mob[k].id = class_; diff --git a/src/map/npc.c b/src/map/npc.c index 8ecefb5a0..38ba9ae41 100644 --- a/src/map/npc.c +++ b/src/map/npc.c @@ -3683,7 +3683,7 @@ const char* npc_parse_mapflag(char* w1, char* w2, char* w3, char* w4, const char map->list[m].save.map = 0; map->list[m].save.x = -1; map->list[m].save.y = -1; - } else if (sscanf(w4, "%31[^,],%d,%d", savemap, &savex, &savey) == 3) { + } else if (w4 && sscanf(w4, "%31[^,],%d,%d", savemap, &savex, &savey) == 3) { map->list[m].save.map = mapindex->name2id(savemap); map->list[m].save.x = savex; map->list[m].save.y = savey; diff --git a/src/map/pc.c b/src/map/pc.c index 10b464570..6e9cc1e7a 100644 --- a/src/map/pc.c +++ b/src/map/pc.c @@ -886,11 +886,12 @@ int pc_isequip(struct map_session_data *sd,int n) item = sd->inventory_data[n]; - if(pc_has_permission(sd, PC_PERM_USE_ALL_EQUIPMENT)) - return 1; - if(item == NULL) return 0; + + if(pc_has_permission(sd, PC_PERM_USE_ALL_EQUIPMENT)) + return 1; + if(item->elv && sd->status.base_level < (unsigned int)item->elv){ clif->msg(sd, 0x6ED); return 0; @@ -9026,20 +9027,18 @@ int pc_equipitem(struct map_session_data *sd,int n,int req_pos) iflag = sd->npc_item_flag; /* check for combos (MUST be before status_calc_pc) */ - if ( id ) { - if( id->combos_count ) - pc->checkcombo(sd,id); - if(itemdb_isspecial(sd->status.inventory[n].card[0])) - ; //No cards - else { - for( i = 0; i < id->slot; i++ ) { - struct item_data *data; - if (!sd->status.inventory[n].card[i]) - continue; - if ( ( data = itemdb->exists(sd->status.inventory[n].card[i]) ) != NULL ) { - if( data->combos_count ) - pc->checkcombo(sd,data); - } + if( id->combos_count ) + pc->checkcombo(sd,id); + if(itemdb_isspecial(sd->status.inventory[n].card[0])) + ; //No cards + else { + for( i = 0; i < id->slot; i++ ) { + struct item_data *data; + if (!sd->status.inventory[n].card[i]) + continue; + if ( ( data = itemdb->exists(sd->status.inventory[n].card[i]) ) != NULL ) { + if( data->combos_count ) + pc->checkcombo(sd,data); } } } diff --git a/src/map/script.c b/src/map/script.c index 2249d53cc..549d3c269 100644 --- a/src/map/script.c +++ b/src/map/script.c @@ -14460,7 +14460,7 @@ BUILDIN(sprintf) { } if(arg>=argc) { ShowError("buildin_sprintf: Not enough arguments passed!\n"); - if(buf) aFree(buf); + aFree(buf); if(buf2) aFree(buf2); StrBuf->Destroy(&final_buf); script_pushconststr(st,""); @@ -14496,7 +14496,7 @@ BUILDIN(sprintf) { } } else { // Unsupported type ShowError("buildin_sprintf: Unknown argument type!\n"); - if(buf) aFree(buf); + aFree(buf); if(buf2) aFree(buf2); StrBuf->Destroy(&final_buf); script_pushconststr(st,""); @@ -14518,7 +14518,7 @@ BUILDIN(sprintf) { script_pushstrcopy(st, StrBuf->Value(&final_buf)); - if(buf) aFree(buf); + aFree(buf); if(buf2) aFree(buf2); StrBuf->Destroy(&final_buf); @@ -14577,7 +14577,7 @@ BUILDIN(sscanf) { if(arg>=argc) { ShowError("buildin_sscanf: Not enough arguments passed!\n"); script_pushint(st, -1); - if(buf) aFree(buf); + aFree(buf); if(ref_str) aFree(ref_str); return false; } @@ -14629,7 +14629,7 @@ BUILDIN(sscanf) { } script_pushint(st, arg); - if(buf) aFree(buf); + aFree(buf); if(ref_str) aFree(ref_str); return true; @@ -18055,7 +18055,10 @@ BUILDIN(npcskill) { skill_level = script_getnum(st, 3); stat_point = script_getnum(st, 4); npc_level = script_getnum(st, 5); - sd = script->rid2sd(st); + + if( !(sd = script->rid2sd(st)) ) + return false; + nd = (struct npc_data *)map->id2bl(sd->npc_id); if (stat_point > battle_config.max_third_parameter) { @@ -18066,7 +18069,7 @@ BUILDIN(npcskill) { ShowError("npcskill: level exceeded maximum of %d.\n", MAX_LEVEL); return false; } - if (sd == NULL || nd == NULL) { + if (nd == NULL) { return false; } @@ -18977,7 +18980,8 @@ BUILDIN(tradertype) { } #endif - nd->u.scr.shop->type = type; + if( nd->u.scr.shop ) + nd->u.scr.shop->type = type; return true; } diff --git a/src/map/skill.c b/src/map/skill.c index 8c1d7e1e5..6328959d7 100644 --- a/src/map/skill.c +++ b/src/map/skill.c @@ -2071,7 +2071,7 @@ int skill_blown(struct block_list* src, struct block_list* target, int count, in break; case BL_SKILL: su = (struct skill_unit *)target; - if( su && su->group && (su->group->unit_id == UNT_ANKLESNARE || su->group->unit_id == UNT_REVERBERATION)) + if( su->group && (su->group->unit_id == UNT_ANKLESNARE || su->group->unit_id == UNT_REVERBERATION)) return 0; // ankle snare cannot be knocked back break; } @@ -2362,9 +2362,11 @@ int skill_attack(int attack_type, struct block_list* src, struct block_list *dsr party->skill_check(sd, sd->status.party_id, MO_COMBOFINISH, skill_lv); if (pc->checkskill(sd, CH_TIGERFIST) > 0 && sd->spiritball > 0) combo=1; + /* Fall through */ case CH_TIGERFIST: if (!combo && pc->checkskill(sd, CH_CHAINCRUSH) > 0 && sd->spiritball > 1) combo=1; + /* Fall through */ case CH_CHAINCRUSH: if (!combo && pc->checkskill(sd, MO_EXTREMITYFIST) > 0 && sd->spiritball > 0 && sd->sc.data[SC_EXPLOSIONSPIRITS]) combo=1; @@ -3310,17 +3312,13 @@ int skill_timerskill(int tid, int64 tick, int id, intptr_t data) { case SR_FALLENEMPIRE: case SR_TIGERCANNON: case SR_SKYNETBLOW: - { - struct map_session_data *sd = NULL; - - if( src->type == BL_PC && (sd = ((TBL_PC*)src)) ) { + if( src->type == BL_PC ) { if( distance_xy(src->x, src->y, target->x, target->y) >= 3 ) break; - skill->castend_damage_id(src, target, skl->skill_id, pc->checkskill(sd, skl->skill_id), tick, 0); + skill->castend_damage_id(src, target, skl->skill_id, pc->checkskill(((TBL_PC*)src), skl->skill_id), tick, 0); } break; - } case SC_ESCAPE: if( skl->type < 4+skl->skill_lv ){ clif->skill_damage(src,src,tick,0,0,-30000,1,skl->skill_id,skl->skill_lv,5); @@ -3918,6 +3916,7 @@ int skill_castend_damage_id(struct block_list* src, struct block_list *bl, uint1 case GC_COUNTERSLASH: case GC_ROLLINGCUTTER: flag |= SD_ANIMATION; + /* Fall through */ case LG_MOONSLASHER: case MH_XENO_SLASHER: clif->skill_damage(src,bl,tick, status_get_amotion(src), 0, -30000, 1, skill_id, skill_lv, 6); @@ -4176,6 +4175,7 @@ int skill_castend_damage_id(struct block_list* src, struct block_list *bl, uint1 case SL_SMA: status_change_end(src, SC_SMA_READY, INVALID_TIMER); + /* Fall through */ case SL_STIN: case SL_STUN: if (sd && !battle_config.allow_es_magic_pc && bl->type != BL_MOB) { @@ -5465,7 +5465,7 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin if (exp < 1) exp = 1; } if(jlv > 0 && pc->nextjobexp(dstsd)) { - jexp = (int)((double)dstsd->status.job_exp * (double)lv * (double)battle_config.resurrection_exp / 1000000.); + jexp = (int)((double)dstsd->status.job_exp * (double)jlv * (double)battle_config.resurrection_exp / 1000000.); if (jexp < 1) jexp = 1; } if(exp > 0 || jexp > 0) @@ -9155,6 +9155,7 @@ int skill_castend_nodamage_id(struct block_list *src, struct block_list *bl, uin break; case WM_SIRCLEOFNATURE: flag |= BCT_SELF|BCT_PARTY|BCT_GUILD; + /* Fall through */ case WM_VOICEOFSIREN: if( skill_id != WM_SIRCLEOFNATURE ) flag &= ~BCT_SELF; @@ -12114,7 +12115,7 @@ int skill_unit_onplace_timer(struct skill_unit *src, struct block_list *bl, int6 break; heal = skill->calc_heal(ss,bl,sg->skill_id, sg->skill_lv, true); - if( tsc->data[SC_AKAITSUKI] && heal ) + if( tsc && tsc->data[SC_AKAITSUKI] && heal ) heal = ~heal + 1; clif->skill_nodamage(&src->bl, bl, AL_HEAL, heal, 1); status->heal(bl, heal, 0, 0); @@ -15495,8 +15496,10 @@ int skill_graffitiremover (struct block_list *bl, va_list ap) { nullpo_ret(bl); nullpo_ret(ap); - if(bl->type!=BL_SKILL || (su=(struct skill_unit *)bl) == NULL) + if(bl->type != BL_SKILL) return 0; + + su = ((struct skill_unit *)bl); if((su->group) && (su->group->unit_id == UNT_GRAFFITI)) skill->delunit(su); @@ -15506,14 +15509,12 @@ int skill_graffitiremover (struct block_list *bl, va_list ap) { int skill_greed (struct block_list *bl, va_list ap) { struct block_list *src; - struct map_session_data *sd=NULL; - struct flooritem_data *fitem=NULL; nullpo_ret(bl); nullpo_ret(src = va_arg(ap, struct block_list *)); - if(src->type == BL_PC && (sd=(struct map_session_data *)src) && bl->type==BL_ITEM && (fitem=(struct flooritem_data *)bl)) - pc->takeitem(sd, fitem); + if(src->type == BL_PC && bl->type==BL_ITEM ) + pc->takeitem(((TBL_PC*)src), ((TBL_ITEM*)bl)); return 0; } @@ -15527,9 +15528,12 @@ int skill_detonator(struct block_list *bl, va_list ap) { nullpo_ret(ap); src = va_arg(ap,struct block_list *); - if( bl->type != BL_SKILL || (su = (struct skill_unit *)bl) == NULL || !su->group ) + if( bl->type != BL_SKILL ) return 0; - if( su->group->src_id != src->id ) + + su = (struct skill_unit *)bl; + + if( !su->group || su->group->src_id != src->id ) return 0; unit_id = su->group->unit_id; @@ -16532,9 +16536,8 @@ int skill_unit_timer_sub(DBKey key, DBData *data, va_list ap) { } } - // useless check for !group ? //Don't continue if unit or even group is expired and has been deleted. - if( !group || !su->alive ) + if( !su->alive ) return 0; dissonance = skill->dance_switch(su, 0); diff --git a/src/map/status.c b/src/map/status.c index fbd039fdc..667d52dc2 100644 --- a/src/map/status.c +++ b/src/map/status.c @@ -4170,28 +4170,30 @@ void status_calc_misc(struct block_list *bl, struct status_data *st, int level) st->batk = cap_value(temp, 0, USHRT_MAX); } else st->batk = status->base_atk(bl, st); - if ( st->cri ) + if ( st->cri ) { switch ( bl->type ) { - case BL_MOB: - if ( battle_config.mob_critical_rate != 100 ) - st->cri = st->cri*battle_config.mob_critical_rate / 100; - if ( !st->cri && battle_config.mob_critical_rate ) - st->cri = 10; - break; - case BL_PC: - //Players don't have a critical adjustment setting as of yet. - break; - case BL_MER: -#ifdef RENEWAL - st->matk_min = st->matk_max = status_base_matk_max(st); - st->def2 = st->vit + level / 10 + st->vit / 5; - st->mdef2 = level / 10 + st->int_ / 5; -#endif - default: - if ( battle_config.critical_rate != 100 ) - st->cri = st->cri*battle_config.critical_rate / 100; - if ( !st->cri && battle_config.critical_rate ) - st->cri = 10; + case BL_MOB: + if ( battle_config.mob_critical_rate != 100 ) + st->cri = st->cri*battle_config.mob_critical_rate / 100; + if ( !st->cri && battle_config.mob_critical_rate ) + st->cri = 10; + break; + case BL_PC: + //Players don't have a critical adjustment setting as of yet. + break; + case BL_MER: + #ifdef RENEWAL + st->matk_min = st->matk_max = status_base_matk_max(st); + st->def2 = st->vit + level / 10 + st->vit / 5; + st->mdef2 = level / 10 + st->int_ / 5; + #endif + /* Fall through */ + default: + if ( battle_config.critical_rate != 100 ) + st->cri = st->cri*battle_config.critical_rate / 100; + if ( !st->cri && battle_config.critical_rate ) + st->cri = 10; + } } if ( bl->type&BL_REGEN ) status->calc_regen(bl, st, status->get_regen_data(bl)); @@ -9995,6 +9997,7 @@ int status_change_end_(struct block_list* bl, enum sc_type type, int tid, const status_change_end(src, SC_RG_CCONFINE_M, INVALID_TIMER); } } + /* Fall through */ case SC_RG_CCONFINE_M: if (sce->val2 > 0) { //Caster has been unlocked... nearby chars need to be unlocked. @@ -10610,9 +10613,9 @@ int status_change_timer(int tid, int64 tick, int id, intptr_t data) { case SC_RUWACH: case SC_WZ_SIGHTBLASTER: if(type == SC_WZ_SIGHTBLASTER) { - //Restore trap immunity - if(sce->val4%2) - sce->val4--; + //Restore trap immunity + if(sce->val4%2) + sce->val4--; map->foreachinrange(status->change_timer_sub, bl, sce->val3, BL_CHAR|BL_SKILL, bl, sce, type, tick); } else map->foreachinrange(status->change_timer_sub, bl, sce->val3, BL_CHAR, bl, sce, type, tick); -- cgit v1.2.3-60-g2f50