- Made guild member exp an unsigned int.

- Modified npc_click to receive the bl that was clicked directly. Also cleaned it up so it isn't as easy to crash the server with invalid ids <.<
- Moved npc_checknear to npc_checknear2 and added npc_checknear. The near version receives a bl, checks it's validity, and returns a TBL_NPC object, near2 does the same but doesn't checks for type NPC. The first returns a pointer, the second returns 1 on fail, 0 success.
- Also uncommented various npc_checknear calls in the code, who's idea was to comment them out so you could exploit npcs from afar with custom packets?
- Added overflow checks for bonus settings mdef_rate/def_rate.
- Added missing update of INT after a buf. 
- Small cleanup of how SC_BLEEDING works.
- Fixed party_foreach_samemap invoking the function on the CASTER instead of on the party members.
- Added clif_parse_ActionRequest_sub to handle player commands, is used from npc_click or any other function that needs to "re-route" a player's request.
- Modified clif_parse_NpcClicked to handle the different situations with different bl-objects (attack on players/mobs, click on npcs or mobs with npc attached)


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

Changelog-Trunk.txt

@@ -3,6 +3,18 @@ Date	Added
 AS OF SVN REV. 5091, WE ARE NOW USING TRUNK.  ALL UNTESTED BUGFIXES/FEATURES GO INTO TRUNK.
 IF YOU HAVE A WORKING AND TESTED BUGFIX PUT IT INTO STABLE AS WELL AS TRUNK.
 
+2006/06/12
+	* Made guild member exp an unsigned int. [Skotlex]
+	* Various cleanups to the npc clicking related functions. Should fix quite
+	  some possible crashes from crafted packets. [Skotlex]
+	* Added overflow checks for bonus settings mdef_rate/def_rate. [Skotlex]
+	* Added missing update of INT after a buf.  [Skotlex]
+	* Small cleanup of how SC_BLEEDING works. [Skotlex]
+	* Fixed party_foreach_samemap invoking the function on the CASTER instead
+	  of on the party members. [Skotlex]
+	* Modified clif_parse_NpcClicked to handle the different situations with
+	  different bl-objects (attack on players/mobs, click on npcs or mobs with
+	  npc attached) [Skotlex]
 2006/06/09
 	* [Fixed]
 	  - Compilation warnings on guild.c and int_guild.c [Lance]

src/char/int_guild.c

@@ -1209,7 +1209,7 @@ int mapif_parse_GuildMemberInfoChange(int fd, int guild_id, int account_id, int
 	{
 		unsigned int exp, old_exp=g->member[i].exp;
 		g->member[i].exp=*((unsigned int *)data);
-		if (g->member[i].exp > (signed int)old_exp && old_exp < INT_MAX)
+		if (g->member[i].exp > old_exp)
 		{
 			exp = g->member[i].exp - old_exp;
 			if (guild_exp_rate != 100)

src/char_sql/int_guild.c

@@ -187,7 +187,7 @@ int inter_guild_tosql(struct guild *g,int flag)
 			if(m->account_id) {
 				//Since nothing references guild member table as foreign keys, it's safe to use REPLACE INTO
 				sprintf(tmp_sql,"REPLACE INTO `%s` (`guild_id`,`account_id`,`char_id`,`hair`,`hair_color`,`gender`,`class`,`lv`,`exp`,`exp_payper`,`online`,`position`,`name`) "
-					"VALUES ('%d','%d','%d','%d','%d','%d','%d','%d','%d','%d','%d','%d','%s')",
+					"VALUES ('%d','%d','%d','%d','%d','%d','%d','%d','%u','%d','%d','%d','%s')",
 				       guild_member_db, g->guild_id, m->account_id,m->char_id,
 				       m->hair,m->hair_color,m->gender,
 				       m->class_,m->lv,m->exp,m->exp_payper,m->online,m->position,
@@ -1527,7 +1527,7 @@ int mapif_parse_GuildMemberInfoChange(int fd,int guild_id,int account_id,int cha
 	{	// EXP
 		unsigned int exp, old_exp=g->member[i].exp;
 		g->member[i].exp=*((unsigned int *)data);
-		if (g->member[i].exp > (signed int)old_exp && old_exp < INT_MAX)
+		if (g->member[i].exp > old_exp)
 		{
 			exp = g->member[i].exp - old_exp;
 			if (guild_exp_rate != 100)

src/common/mmo.h

@@ -261,7 +261,8 @@ struct party {
 struct guild_member {
 	int account_id, char_id;
 	short hair,hair_color,gender,class_,lv;
-	int exp,exp_payper;
+	unsigned int exp;
+	int exp_payper;
 	short online,position;
 	int rsv1,rsv2;
 	char name[NAME_LENGTH];

src/map/clif.c

@@ -8804,16 +8804,9 @@ void clif_parse_HowManyConnections(int fd, struct map_session_data *sd) {
 	WFIFOSET(fd,packet_len_table[0xc2]);
 }
 
-/*==========================================
- *
- *------------------------------------------
- */
-void clif_parse_ActionRequest(int fd, struct map_session_data *sd) {
-	unsigned int tick;
+static void clif_parse_ActionRequest_sub(struct map_session_data *sd, int action_type, int target_id, unsigned int tick)
+{
 	unsigned char buf[64];
-	int action_type, target_id;
-	RFIFOHEAD(fd);
-
 	if (pc_isdead(sd)) {
 		clif_clearchar_area(&sd->bl, 1);
 		return;
@@ -8825,14 +8818,9 @@ void clif_parse_ActionRequest(int fd, struct map_session_data *sd) {
 		sd->sc.data[SC_BLADESTOP].timer != -1))
 		return;
 
-	tick = gettick();
-
 	pc_stop_walking(sd, 1);
 	pc_stop_attack(sd);
 
-	target_id = RFIFOL(fd,packet_db[sd->packet_ver][RFIFOW(fd,0)].pos[0]);
-	action_type = RFIFOB(fd,packet_db[sd->packet_ver][RFIFOW(fd,0)].pos[1]);
-
 	if(target_id<0 && -target_id == sd->bl.id) // for disguises [Valaris]
 		target_id = sd->bl.id;
 		
@@ -8895,7 +8883,7 @@ void clif_parse_ActionRequest(int fd, struct map_session_data *sd) {
 			return;
 		}
 		pc_setstand(sd);
-		skill_gangsterparadise(sd, 0); // ギャングスターパラダイス解除 fixed Valaris
+		skill_gangsterparadise(sd, 0); 
 		skill_rest(sd, 0); // TK_HPTIME standing up mode [Dralnu]
 		WBUFW(buf, 0) = 0x8a;
 		WBUFL(buf, 2) = sd->bl.id;
@@ -8905,6 +8893,18 @@ void clif_parse_ActionRequest(int fd, struct map_session_data *sd) {
 	}
 }
 
+/*==========================================
+ *
+ *------------------------------------------
+ */
+void clif_parse_ActionRequest(int fd, struct map_session_data *sd) {
+	clif_parse_ActionRequest_sub(sd,
+		RFIFOB(fd,packet_db[sd->packet_ver][RFIFOW(fd,0)].pos[1]),
+		RFIFOL(fd,packet_db[sd->packet_ver][RFIFOW(fd,0)].pos[0]),
+		gettick()
+	);
+}
+
 /*==========================================
  *
  *------------------------------------------
@@ -9310,6 +9310,7 @@ void clif_parse_UnequipItem(int fd,struct map_session_data *sd)
  */
 void clif_parse_NpcClicked(int fd,struct map_session_data *sd)
 {
+	struct block_list *bl;
 	RFIFOHEAD(fd);
 
 	if(pc_isdead(sd)) {
@@ -9320,7 +9321,23 @@ void clif_parse_NpcClicked(int fd,struct map_session_data *sd)
 	if (clif_cant_act(sd))
 		return;
 
-	npc_click(sd,RFIFOL(fd,2));
+	bl = map_id2bl(RFIFOL(fd,2));
+	if (!bl) return;
+	switch (bl->type) {
+		case BL_MOB:
+			if (((TBL_MOB *)bl)->nd)
+				npc_click(sd, bl);
+			else
+				clif_parse_ActionRequest_sub(sd, 0x07, bl->id, gettick());
+			break;
+		case BL_PC:
+			clif_parse_ActionRequest_sub(sd, 0x07, bl->id, gettick());
+			break;
+		case BL_NPC:
+			npc_click(sd,bl);
+			break;
+	}
+	return;
 }
 
 /*==========================================

src/map/guild.c

@@ -367,7 +367,7 @@ int guild_payexp_timer_sub(DBKey dataid, void *data, va_list ap)
 		return 0;
 	}
 
-	if ((long long)g->member[i].exp > (long long)(UINT_MAX - c->exp))
+	if (g->member[i].exp > UINT_MAX - c->exp)
 		g->member[i].exp = UINT_MAX;
 	else
 		g->member[i].exp+= c->exp;

src/map/npc.c

@@ -962,7 +962,7 @@ int npc_touch_areanpc(struct map_session_data *sd,int m,int x,int y)
 
 			if( npc_event(sd,name,0)>0 ) {
 				pc_stop_walking(sd,1); //Make it stop walking!
-				npc_click(sd,map[m].npc[i]->bl.id);
+				npc_click(sd,&(map[m].npc[i]->bl));
 			}
 			//aFree(name);
 			break;
@@ -1008,36 +1008,51 @@ int npc_touch_areanpc2(struct block_list *bl)
  * 近くかどうかの判定
  *------------------------------------------
  */
-int npc_checknear(struct map_session_data *sd,int id)
+int npc_checknear2(struct map_session_data *sd,struct block_list *bl)
 {
-	struct npc_data *nd;
-
-	nullpo_retr(0, sd);
-
-	if(sd->state.using_fake_npc)
+	nullpo_retr(1, sd);
+	if(bl == NULL) return 1;
+	
+	if(sd->state.using_fake_npc && sd->npc_id == bl->id)
 		return 0;
 
-	nd=(struct npc_data *)map_id2bl(id);
-	if (nd==NULL) {
-		if (battle_config.error_log)
-			ShowWarning("no such npc : %d\n",id);
-		return 1;
-	}
-	if (nd->bl.type!=BL_NPC) //Disguised character or something else...
-		return 1;
+//	if (bl->type!=BL_NPC) //Disguised character or something else...
+//		return 1;
 
-	if (nd->class_<0)	// イベント系は常にOK
+	if (status_get_class(bl)<0) //Class-less npc, enable click from anywhere.
 		return 0;
 
-	// エリア判定
-	if (nd->bl.m!=sd->bl.m ||
-	   nd->bl.x<sd->bl.x-AREA_SIZE-1 || nd->bl.x>sd->bl.x+AREA_SIZE+1 ||
-	   nd->bl.y<sd->bl.y-AREA_SIZE-1 || nd->bl.y>sd->bl.y+AREA_SIZE+1)
+	if (bl->m!=sd->bl.m ||
+	   bl->x<sd->bl.x-AREA_SIZE-1 || bl->x>sd->bl.x+AREA_SIZE+1 ||
+	   bl->y<sd->bl.y-AREA_SIZE-1 || bl->y>sd->bl.y+AREA_SIZE+1)
 		return 1;
 
 	return 0;
 }
 
+TBL_NPC *npc_checknear(struct map_session_data *sd,struct block_list *bl)
+{
+	struct npc_data *nd;
+
+	nullpo_retr(NULL, sd);
+	if(bl == NULL) return NULL;
+	if(bl->type != BL_NPC) return NULL;
+	nd = (TBL_NPC*)bl;
+
+	if(sd->state.using_fake_npc && sd->npc_id == bl->id)
+		return nd;
+
+	if (nd->class_<0) //Class-less npc, enable click from anywhere.
+		return nd;
+
+	if (bl->m!=sd->bl.m ||
+	   bl->x<sd->bl.x-AREA_SIZE-1 || bl->x>sd->bl.x+AREA_SIZE+1 ||
+	   bl->y<sd->bl.y-AREA_SIZE-1 || bl->y>sd->bl.y+AREA_SIZE+1)
+		return nd;
+
+	return NULL;
+}
+
 /*==========================================
  * NPCのオープンチャット発言
  *------------------------------------------
@@ -1060,11 +1075,9 @@ int npc_globalmessage(const char *name,char *mes)
  * クリック時のNPC処理
  *------------------------------------------
  */
-int npc_click(struct map_session_data *sd,int id)
+int npc_click(struct map_session_data *sd,struct block_list *bl)
 {
 	struct npc_data *nd = NULL;
-	TBL_MOB *md = NULL;
-	int tick = 0;
 
 	nullpo_retr(1, sd);
 
@@ -1074,53 +1087,32 @@ int npc_click(struct map_session_data *sd,int id)
 		return 1;
 	}
 
-	if(id < 0){
-		id = -id;
-	}
-
-	nd=(struct npc_data *)map_id2bl(id);
-
-	switch(nd->bl.type){
+	if(!bl) return 1;
+	switch(bl->type){
 		case BL_MOB:
-			md = ((TBL_MOB *)nd);
-			if(md->nd){
-				if(sd->bl.m == md->bl.m && distance_bl(&sd->bl, &md->bl) <= AREA_SIZE)
-					sd->npc_pos = run_script(md->nd->u.scr.script,0,sd->bl.id,id);
-				return 0;
-			}
-		case BL_PC:
-			tick = gettick();
-			if (sd->sc.option&OPTION_HIDE || sd->sc.option&OPTION_WEDDING || sd->vd.class_ == JOB_XMAS)
-				return 0;
-
-			if (!battle_config.sdelay_attack_enable && pc_checkskill(sd, SA_FREECAST) <= 0) {
-				if (DIFF_TICK(tick, sd->ud.canact_tick) < 0) {
-					clif_skill_fail(sd, 1, 4, 0);
-					return 0;
-				}
-			}
-			if (sd->invincible_timer != -1)
-				pc_delinvincibletimer(sd);
-
-			sd->idletime = tick;
-			unit_attack(&sd->bl, id, 1);
-			return 0;
+			if (npc_checknear2(sd,bl))
+				return 1;
+			if((nd = ((TBL_MOB *)bl)->nd) == NULL)
+				return 1;
+			break;
+		case BL_NPC:
+			if ((nd = npc_checknear(sd,bl)) == NULL)
+				return 1;
+			//Hidden/Disabled npc.
+			if (nd->class_ < 0 || nd->sc.option&OPTION_INVISIBLE)
+				return 1;
+			break;
+		default:
+			return 1;
 	}
 
-	if (npc_checknear(sd,id))
-		return 1;
-
-	//Hidden/Disabled npc.
-	if (nd->class_ < 0 || nd->sc.option&OPTION_INVISIBLE)
-		return 1;
-
 	switch(nd->bl.subtype) {
 	case SHOP:
-		clif_npcbuysell(sd,id);
+		clif_npcbuysell(sd,nd->bl.id);
 		npc_event_dequeue(sd);
 		break;
 	case SCRIPT:
-		sd->npc_pos=run_script(nd->u.scr.script,0,sd->bl.id,id);
+		sd->npc_pos=run_script(nd->u.scr.script,0,sd->bl.id,nd->bl.id);
 		break;
 	}
 
@@ -1133,27 +1125,22 @@ int npc_click(struct map_session_data *sd,int id)
  */
 int npc_scriptcont(struct map_session_data *sd,int id)
 {
-	struct npc_data *nd;
-
 	nullpo_retr(1, sd);
 
 	if (id!=sd->npc_id){
 		ShowWarning("npc_scriptcont: sd->npc_id (%d) is not id (%d).\n", sd->npc_id, id);
 		return 1;
 	}
-
-	if(sd->npc_id != fake_nd->bl.id){ // Not item script
-		if (npc_checknear(sd,id)){
+	
+	if(id != fake_nd->bl.id) { // Not item script
+		struct npc_data *nd;
+		if ((nd = npc_checknear(sd,map_id2bl(id))) == NULL){
 			ShowWarning("npc_scriptcont: failed npc_checknear test.\n");
 			return 1;
 		}
-
-		nd=(struct npc_data *)map_id2bl(id);
-
 		sd->npc_pos=run_script(nd->u.scr.script,sd->npc_pos,sd->bl.id,id);
-	} else { // Item script, continue execution...
+	} else // Item script, continue execution...
 		sd->npc_pos=run_script(sd->npc_scriptroot,sd->npc_pos,sd->bl.id,id);
-	}
 
 	return 0;
 }
@@ -1168,14 +1155,14 @@ int npc_buysellsel(struct map_session_data *sd,int id,int type)
 
 	nullpo_retr(1, sd);
 
-	if (npc_checknear(sd,id))
+	if ((nd = npc_checknear(sd,map_id2bl(id))) == NULL)
 		return 1;
-
-	nd=(struct npc_data *)map_id2bl(id);
+	
 	if (nd->bl.subtype!=SHOP) {
 		if (battle_config.error_log)
 			ShowError("no such shop npc : %d\n",id);
-		//sd->npc_id=0;
+		if (sd->npc_id == id)
+			sd->npc_id=0;
 		return 1;
 	}
 	if (nd->sc.option&OPTION_INVISIBLE)	// 無効化されている
@@ -1203,10 +1190,9 @@ int npc_buylist(struct map_session_data *sd,int n,unsigned short *item_list)
 	nullpo_retr(3, sd);
 	nullpo_retr(3, item_list);
 
-	//if (npc_checknear(sd,sd->npc_shopid))
-	//	return 3;
+	if ((nd = npc_checknear(sd,map_id2bl(sd->npc_shopid))) == NULL)
+		return 3;
 
-	nd=(struct npc_data*)map_id2bl(sd->npc_shopid);
 	if (nd->bl.subtype!=SHOP)
 		return 3;
 
@@ -1298,12 +1284,10 @@ int npc_selllist(struct map_session_data *sd,int n,unsigned short *item_list)
 	nullpo_retr(1, sd);
 	nullpo_retr(1, item_list);
 
-	nd = (struct npc_data *)map_id2bl(sd->npc_shopid);
-	if (!nd) return 1;
+	if ((nd = npc_checknear(sd,map_id2bl(sd->npc_shopid))) == NULL)
+		return 1;
 	nd = nd->master_nd; //For OnSell triggers.
-	
-	//if (npc_checknear(sd,sd->npc_shopid))
-	//	return 1;
+
 	for(i=0,z=0;i<n;i++) {
 		int nameid, idx, qty;
 		idx = item_list[i*2]-2;

src/map/npc.h

@@ -41,9 +41,10 @@ int npc_timer_event(const unsigned char *eventname);				// Added by RoVeRT
 int npc_command(struct map_session_data *sd,const unsigned char *npcname,char *command);
 int npc_touch_areanpc(struct map_session_data *,int,int,int);
 int npc_touch_areanpc2(struct block_list *bl); // [Skotlex]
-int npc_click(struct map_session_data *,int);
+int npc_click(struct map_session_data *sd,struct block_list *bl);
 int npc_scriptcont(struct map_session_data *,int);
-int npc_checknear(struct map_session_data *,int);
+TBL_NPC *npc_checknear(struct map_session_data *sd,struct block_list *bl);
+int npc_checknear2(struct map_session_data *sd,struct block_list *bl);
 int npc_buysellsel(struct map_session_data *,int,int);
 int npc_buylist(struct map_session_data *,int,unsigned short *);
 int npc_selllist(struct map_session_data *,int,unsigned short *);

src/map/party.c

@@ -823,7 +823,7 @@ int party_foreachsamemap(int (*func)(struct block_list*,va_list),struct map_sess
 			(psd->bl.x<x0 || psd->bl.y<y0 ||
 			 psd->bl.x>x1 || psd->bl.y>y1 ) )
 			continue;
-		list[blockcount++]=&sd->bl; 
+		list[blockcount++]=&psd->bl; 
 	}
 
 	map_freeblock_lock();

src/map/status.c

@@ -1874,8 +1874,10 @@ int status_calc_pc(struct map_session_data* sd,int first)
 // ----- EQUIPMENT-DEF CALCULATION -----
 
 	// Apply relative modifiers from equipment
-	if(sd->def_rate != 100)
-		status->def = status->def * sd->def_rate/100;
+	if(sd->def_rate != 100) {
+		i =  status->def * sd->def_rate/100;
+		status->def = cap_value(i, CHAR_MIN, CHAR_MAX);
+	}
 
 	if (!battle_config.weapon_defense_type && status->def > battle_config.max_def)
 	{
@@ -1891,13 +1893,15 @@ int status_calc_pc(struct map_session_data* sd,int first)
 // ----- EQUIPMENT-MDEF CALCULATION -----
 
 	// Apply relative modifiers from equipment
-	if(sd->mdef_rate != 100)
-		status->mdef = status->mdef * sd->mdef_rate/100;
+	if(sd->mdef_rate != 100) {
+		i =  status->mdef * sd->mdef_rate/100;
+		status->mdef = cap_value(i, CHAR_MIN, CHAR_MAX);
+	}
 
 	if (!battle_config.magic_defense_type && status->mdef > battle_config.max_def)
 	{
 		status->mdef2 += battle_config.over_def_bonus*(status->mdef -battle_config.max_def);
-		status->mdef = (unsigned char)battle_config.max_def;
+		status->mdef = (signed char)battle_config.max_def;
 	}
 	
 // ----- WALKING SPEED CALCULATION -----
@@ -2396,6 +2400,8 @@ void status_calc_bl_sub_pc(struct map_session_data *sd, unsigned long flag)
 		clif_updatestatus(sd,SP_AGI);
 	if(flag&SCB_VIT)
 		clif_updatestatus(sd,SP_VIT);
+	if(flag&SCB_INT)
+		clif_updatestatus(sd,SP_INT);
 	if(flag&SCB_DEX)
 		clif_updatestatus(sd,SP_DEX);
 	if(flag&SCB_LUK)
@@ -4560,7 +4566,8 @@ int status_change_start(struct block_list *bl,int type,int rate,int val1,int val
 			clif_emotion(bl,1);
 			break;
 		case SC_BLEEDING:
-			val4 = tick;
+			val4 = tick/10000;
+			if (!val4) val4 = 1;
 			tick = 10000;
 			break;
 
@@ -5883,7 +5890,7 @@ int status_change_timer(int tid, unsigned int tick, int id, int data)
 		// - 10酴���HP�术岽
 		// - 躞�����?�旃匝�������?妄�峒���
 		// To-do: bleeding effect increases damage taken?
-		if ((sc->data[type].val4 -= 10000) >= 0) {
+		if ((--sc->data[type].val4) >= 0) {
 			status_fix_damage(NULL, bl, rand()%600 + 200, 0);
 			if (status_isdead(bl))
 				break;

src/map/unit.c

@@ -1080,7 +1080,7 @@ int unit_attack(struct block_list *src,int target_id,int type)
 	}
 
 	if(src->type == BL_PC && target->type==BL_NPC) { // monster npcs [Valaris]
-		npc_click((TBL_PC*)src,target_id); // submitted by leinsirk10 [Celest]
+		npc_click((TBL_PC*)src,target); // submitted by leinsirk10 [Celest]
 		return 0;
 	}