npc: Deregister events with `ev_db` on free
In serverdata@c3b7fe59, magic-knuckles
made use of temporary copies of the
spell NPC via puppet
/destroy
to provide additional functionality. This
implementation was correct with respect to tmwa behaviour as documented.
However, puppet
and destroy
doesn't quite return the server to a clean
prior state. In builtin_puppet
, events with the fresh new copy of NPC are
registered into the global ev_db
map to track named events, such as
OnDischarge
, which might be triggered later. However, these registrations
are not reversed with destroy
, leaving ev_db
to retain references to now
invalid and destroyed NPCs.
serverdata@c3b7fe59 revealed this oversight through an intersection of rare conditions:
- An NPC that is routinely cloned and destroyed
- Having a variety of events
- Including one that can be triggered during a search for the event over ALL
NPCs, when
#discharge
is cast.
To reproduce, compile with -fsanitize=address -fsanitize=undefined
to more
reliably catch use of invalid memory, cast magic-knuckles
, log out the
character which cast magic-knuckles
to destroy the NPC, log back in, then
cast #discharge
. The server will then crash.
Special thanks to SystemError for testing out this theory. Currently lacking a test environment of my own, this would not have been possible without his patience and diligence.