Issue Information
-
#004950
-
0 - None Assigned
-
Fixed
Issue Confirmations
-
Yes (0)No (0)
0
Originally posted by theultramage
http://www.eathena.w...er&showbug=4950
in battle_weapon_attack():
Calling status_zap() can kill the user, in which case skill_attack() will execute with a dead unit.
Now consider what happens when a player clone does this. Upon death, it is immediately unit_free'd, which involves calling
So not only does the clone have partially removed data, it also no longer has its mobdb entry, and more importantly, its view data is set to NULL. This ultimately leads to a crash in status_get_class(), which expects a non-null md->vd (crash is in battle_calc_weapon_attack).
To reproduce, just make a strong enough paladin with martyr's reckoning, and damage its health by at least 10% (then clones start casting self skills).
http://www.eathena.w...er&showbug=4950
in battle_weapon_attack():
CODE
if (sc->data[SC_SACRIFICE])
{
...
status_zap(src, sstatus->max_hp*9/100, 0);//Damage to self is always 9%
return (damage_lv)skill_attack(BF_WEAPON,src,src,target,PA_SACRIFICE,skilllv,tick,0);
{
...
status_zap(src, sstatus->max_hp*9/100, 0);//Damage to self is always 9%
return (damage_lv)skill_attack(BF_WEAPON,src,src,target,PA_SACRIFICE,skilllv,tick,0);
Calling status_zap() can kill the user, in which case skill_attack() will execute with a dead unit.
Now consider what happens when a player clone does this. Upon death, it is immediately unit_free'd, which involves calling
CODE
int mob_clone_delete(struct mob_data *md)
{
const int class_ = md->class_;
if (class_ >= MOB_CLONE_START && class_ < MOB_CLONE_END && mob_db_data[class_]!=NULL) {
aFree(mob_db_data[class_]);
mob_db_data[class_]=NULL;
//Clear references to the db
md->db = mob_dummy;
md->vd = NULL;}
{
const int class_ = md->class_;
if (class_ >= MOB_CLONE_START && class_ < MOB_CLONE_END && mob_db_data[class_]!=NULL) {
aFree(mob_db_data[class_]);
mob_db_data[class_]=NULL;
//Clear references to the db
md->db = mob_dummy;
md->vd = NULL;}
So not only does the clone have partially removed data, it also no longer has its mobdb entry, and more importantly, its view data is set to NULL. This ultimately leads to a crash in status_get_class(), which expects a non-null md->vd (crash is in battle_calc_weapon_attack).
To reproduce, just make a strong enough paladin with martyr's reckoning, and damage its health by at least 10% (then clones start casting self skills).
Originally posted by xazax
What about calculating skill_attack first, storing it's return value. Call status_zap after that, and return the stored dmg.
What about calculating skill_attack first, storing it's return value. Call status_zap after that, and return the stored dmg.
Originally posted by Ind
but would it be worth the effort? rewriting how the whole thing works prior to fixing this custom-feature-made bug, what if we delayed it's 'free'? the mob_dead already delays "client death" of clones, what if we added a timer to release it? something like 0.5s would be more than enough for the rest of the calc proceed in order for it to not crashWhat about calculating skill_attack first, storing it's return value. Call status_zap after that, and return the stored dmg.
Originally posted by xazax
I fix it my way, because it involves less serious modifications. However if we find similar crash bugs, than your solution should be applied since it would fix all of them globally. Fixed in [rev=15277]
I fix it my way, because it involves less serious modifications. However if we find similar crash bugs, than your solution should be applied since it would fix all of them globally. Fixed in [rev=15277]
