skill_trap_splash nullpo error
Warning! This is the old Hercules bugtracker archive, and may not reflect the current state of Hercules. The current bugtracker is on GitHub Issues.
Issue information
Issue ID
#4426
Status
Fixed
Severity
Medium
Started
Hercules Elf Bot
Sep 9, 2010 3:36
Last Post
Hercules Elf Bot
Sep 9, 2010 3:36
Confirmation
Hercules Elf Bot - Sep 9, 2010 3:36
Originally posted by [b]theultramage[/b]
http://www.eathena.ws/board/index.php?autocom=bugtracker&showbug=4426
There exists an unintended scenario where skill_trap_splash() will execute on a skill unit that has no group (array of ground cells) associated with it. Since this is an unexpected event, the only thing handling it is a nullpo_retr() check - which will however get disabled in non-debug builds. The consequence is that the followup code will attempt to dereference a null pointer and crash the mapserver.
Here are the steps to reproduce (figured out thanks to digos):
nullpo_foreach_splash_warp.txt ( 384bytes ) Number of downloads: 34
Now what happens is that the splash code will process mobs one by one. The first one will die and trigger the warp event. This will move the player to a different map, and doing so will wipe all land skills belonging to the player. Then the next mob will be processed for splash damage; however, the skill unit group is no longer there.
Since the same scenario ought to be happening with AoE skills like Stormgust, I checked skill_unit_timer_sub_onplace()... and from there copied the following lines:
This post has been edited by theultramage: Sep 8 2010, 08:36 PM
http://www.eathena.ws/board/index.php?autocom=bugtracker&showbug=4426
There exists an unintended scenario where skill_trap_splash() will execute on a skill unit that has no group (array of ground cells) associated with it. Since this is an unexpected event, the only thing handling it is a nullpo_retr() check - which will however get disabled in non-debug builds. The consequence is that the followup code will attempt to dereference a null pointer and crash the mapserver.
Here are the steps to reproduce (figured out thanks to digos):
- place trap-like skill unit on ground (hunter traps, fire pillar).
- have multiple mobs get killed by this trap at the same time.
- have the player warped to a different map by an OnDead event attached to each of these mobs.
nullpo_foreach_splash_warp.txt ( 384bytes ) Number of downloads: 34Now what happens is that the splash code will process mobs one by one. The first one will die and trigger the warp event. This will move the player to a different map, and doing so will wipe all land skills belonging to the player. Then the next mob will be processed for splash damage; however, the skill unit group is no longer there.
Since the same scenario ought to be happening with AoE skills like Stormgust, I checked skill_unit_timer_sub_onplace()... and from there copied the following lines:
CODE
static int skill_trap_splash (struct block_list *bl, va_list ap)
{
...
+ if( !unit->alive || bl->prev == NULL )
+ return 0;
nullpo_retr(0, sg = unit->group);
nullpo_retr(0, ss = map_id2bl(sg->src_id));
{
...
+ if( !unit->alive || bl->prev == NULL )
+ return 0;
nullpo_retr(0, sg = unit->group);
nullpo_retr(0, ss = map_id2bl(sg->src_id));
This post has been edited by theultramage: Sep 8 2010, 08:36 PM