From bf31d98414a7374e8900a7c52f1f2eda279a1c76 Mon Sep 17 00:00:00 2001 From: Thomas Arp <357770+welcor@users.noreply.github.com> Date: Sun, 19 Jan 2020 14:40:33 +0100 Subject: [PATCH 1/5] Make sure all followers are free'd before freeing the character list Otherwise, the followers structs will point to free'd memory and the stop_follower call will attempt to dereference a free'd characters' followers list. --- src/db.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/db.c b/src/db.c index 002dcc4..746b3a2 100644 --- a/src/db.c +++ b/src/db.c @@ -501,15 +501,22 @@ static void free_extra_descriptions(struct extra_descr_data *edesc) void destroy_db(void) { ssize_t cnt, itr; - struct char_data *chtmp; + struct char_data *chtmp, *i = character_list; struct obj_data *objtmp; /* Active Mobiles & Players */ + while (i) { + chtmp = i; + i = i->next; + + if (chtmp->master) + stop_follower(chtmp); + } + while (character_list) { chtmp = character_list; character_list = character_list->next; - if (chtmp->master) - stop_follower(chtmp); + free_char(chtmp); } From d5a11618f1a1a3fa19662d4717c04081f52ab2a2 Mon Sep 17 00:00:00 2001 From: Thomas Arp <357770+welcor@users.noreply.github.com> Date: Sun, 1 Mar 2020 01:19:06 +0100 Subject: [PATCH 2/5] Remove crash bug when purging a dropped item in a wtrigger. We're leveraging the lookup table, because it's a safer way to see if an object has been free'd than looking at the object itself (which while it may work may just as well fail). Fixes #83 --- src/act.item.c | 33 ++++++++++++++++++++++----------- src/dg_scripts.c | 24 ++++++++++++++++-------- src/dg_scripts.h | 2 ++ 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/act.item.c b/src/act.item.c index 3467659..e0baf18 100644 --- a/src/act.item.c +++ b/src/act.item.c @@ -52,11 +52,12 @@ static void wear_message(struct char_data *ch, struct obj_data *obj, int where); static void perform_put(struct char_data *ch, struct obj_data *obj, struct obj_data *cont) { + long object_id = obj_script_id(obj); if (!drop_otrigger(obj, ch)) return; - if (!obj) /* object might be extracted by drop_otrigger */ + if (!has_obj_by_uid_in_lookup_table(object_id)) /* object might be extracted by drop_otrigger */ return; if ((GET_OBJ_VAL(cont, 0) > 0) && @@ -409,24 +410,27 @@ static void perform_drop_gold(struct char_data *ch, int amount, byte mode, room_ WAIT_STATE(ch, PULSE_VIOLENCE); /* to prevent coin-bombing */ obj = create_money(amount); if (mode == SCMD_DONATE) { - send_to_char(ch, "You throw some gold into the air where it disappears in a puff of smoke!\r\n"); - act("$n throws some gold into the air where it disappears in a puff of smoke!", - FALSE, ch, 0, 0, TO_ROOM); - obj_to_room(obj, RDR); - act("$p suddenly appears in a puff of orange smoke!", 0, 0, obj, 0, TO_ROOM); + send_to_char(ch, "You throw some gold into the air where it disappears in a puff of smoke!\r\n"); + act("$n throws some gold into the air where it disappears in a puff of smoke!", + FALSE, ch, 0, 0, TO_ROOM); + obj_to_room(obj, RDR); + act("$p suddenly appears in a puff of orange smoke!", 0, 0, obj, 0, TO_ROOM); } else { char buf[MAX_STRING_LENGTH]; + long object_id = obj_script_id(obj); if (!drop_wtrigger(obj, ch)) { - extract_obj(obj); + if (has_obj_by_uid_in_lookup_table(object_id)) + extract_obj(obj); + return; } - snprintf(buf, sizeof(buf), "$n drops %s.", money_desc(amount)); - act(buf, TRUE, ch, 0, 0, TO_ROOM); + snprintf(buf, sizeof(buf), "$n drops %s.", money_desc(amount)); + act(buf, TRUE, ch, 0, 0, TO_ROOM); - send_to_char(ch, "You drop some gold.\r\n"); - obj_to_room(obj, IN_ROOM(ch)); + send_to_char(ch, "You drop some gold.\r\n"); + obj_to_room(obj, IN_ROOM(ch)); } } else { char buf[MAX_STRING_LENGTH]; @@ -447,13 +451,20 @@ static int perform_drop(struct char_data *ch, struct obj_data *obj, { char buf[MAX_STRING_LENGTH]; int value; + long object_id = obj_script_id(obj); if (!drop_otrigger(obj, ch)) return 0; + if (!has_obj_by_uid_in_lookup_table(object_id)) + return 0; // item was extracted by script + if ((mode == SCMD_DROP) && !drop_wtrigger(obj, ch)) return 0; + if (!has_obj_by_uid_in_lookup_table(object_id)) + return 0; // item was extracted by script + if (OBJ_FLAGGED(obj, ITEM_NODROP) && !PRF_FLAGGED(ch, PRF_NOHASSLE)) { snprintf(buf, sizeof(buf), "You can't %s $p, it must be CURSED!", sname); act(buf, FALSE, ch, obj, 0, TO_CHAR); diff --git a/src/dg_scripts.c b/src/dg_scripts.c index bbd5ca9..709f5f5 100644 --- a/src/dg_scripts.c +++ b/src/dg_scripts.c @@ -2994,12 +2994,18 @@ void init_lookup_table(void) } } -static struct char_data *find_char_by_uid_in_lookup_table(long uid) +static struct lookup_table_t *find_element_by_uid_in_lookup_table(long uid) { int bucket = (int) (uid & (BUCKET_COUNT - 1)); struct lookup_table_t *lt = &lookup_table[bucket]; for (;lt && lt->uid != uid ; lt = lt->next) ; + return lt; +} + +static struct char_data *find_char_by_uid_in_lookup_table(long uid) +{ + struct lookup_table_t *lt = find_element_by_uid_in_lookup_table(uid); if (lt) return (struct char_data *)(lt->c); @@ -3010,10 +3016,7 @@ static struct char_data *find_char_by_uid_in_lookup_table(long uid) static struct obj_data *find_obj_by_uid_in_lookup_table(long uid) { - int bucket = (int) (uid & (BUCKET_COUNT - 1)); - struct lookup_table_t *lt = &lookup_table[bucket]; - - for (;lt && lt->uid != uid ; lt = lt->next) ; + struct lookup_table_t *lt = find_element_by_uid_in_lookup_table(uid); if (lt) return (struct obj_data *)(lt->c); @@ -3022,6 +3025,13 @@ static struct obj_data *find_obj_by_uid_in_lookup_table(long uid) return NULL; } +int has_obj_by_uid_in_lookup_table(long uid) +{ + struct lookup_table_t *lt = find_element_by_uid_in_lookup_table(uid); + + return lt != NULL; +} + void add_to_lookup_table(long uid, void *c) { int bucket = (int) (uid & (BUCKET_COUNT - 1)); @@ -3054,9 +3064,7 @@ void remove_from_lookup_table(long uid) if (uid == 0) return; - for (;lt;lt = lt->next) - if (lt->uid == uid) - flt = lt; + flt = find_element_by_uid_in_lookup_table(uid); if (flt) { for (lt = &lookup_table[bucket];lt->next != flt;lt = lt->next) diff --git a/src/dg_scripts.h b/src/dg_scripts.h index 838b3f6..e860f02 100644 --- a/src/dg_scripts.h +++ b/src/dg_scripts.h @@ -449,6 +449,8 @@ void wld_command_interpreter(room_data *room, char *argument); // id helpers extern long char_script_id(char_data *ch); extern long obj_script_id(obj_data *obj); +extern int has_obj_by_uid_in_lookup_table(long uid); + #define room_script_id(room) ((long)(room)->number + ROOM_ID_BASE) #endif /* _DG_SCRIPTS_H_ */ From 53870eba5d14eb101146263974e3df81cf3676e7 Mon Sep 17 00:00:00 2001 From: Thomas Arp <357770+welcor@users.noreply.github.com> Date: Sun, 1 Mar 2020 01:27:57 +0100 Subject: [PATCH 3/5] Added further failsafes to prevent dereferencing free'd objects "obj" variable is not updated here, so we must lookup to see if it has been free'd in script_driver(). Fixes #83 --- src/dg_triggers.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/dg_triggers.c b/src/dg_triggers.c index 1b6db8f..5a54d17 100644 --- a/src/dg_triggers.c +++ b/src/dg_triggers.c @@ -462,6 +462,7 @@ int receive_mtrigger(char_data *ch, char_data *actor, obj_data *obj) trig_data *t; char buf[MAX_INPUT_LENGTH]; int ret_val; + long object_id = obj_script_id(obj); if (!SCRIPT_CHECK(ch, MTRIG_RECEIVE) || AFF_FLAGGED(ch, AFF_CHARM)) return 1; @@ -471,9 +472,10 @@ int receive_mtrigger(char_data *ch, char_data *actor, obj_data *obj) (rand_number(1, 100) <= GET_TRIG_NARG(t))){ ADD_UID_VAR(buf, t, char_script_id(actor), "actor", 0); - ADD_UID_VAR(buf, t, obj_script_id(obj), "object", 0); + ADD_UID_VAR(buf, t, object_id, "object", 0); ret_val = script_driver(&ch, t, MOB_TRIGGER, TRIG_NEW); - if (DEAD(actor) || DEAD(ch) || obj->carried_by != actor) + // check for purged item before dereferencing. + if (DEAD(actor) || DEAD(ch) || !has_obj_by_uid_in_lookup_table(object_id) || obj->carried_by != actor) return 0; else return ret_val; @@ -1118,6 +1120,7 @@ int drop_wtrigger(obj_data *obj, char_data *actor) trig_data *t; char buf[MAX_INPUT_LENGTH]; int ret_val; + long object_id = obj_script_id(obj); if (!actor || !SCRIPT_CHECK(&world[IN_ROOM(actor)], WTRIG_DROP)) return 1; @@ -1127,9 +1130,10 @@ int drop_wtrigger(obj_data *obj, char_data *actor) if (TRIGGER_CHECK(t, WTRIG_DROP) && (rand_number(1, 100) <= GET_TRIG_NARG(t))) { ADD_UID_VAR(buf, t, char_script_id(actor), "actor", 0); - ADD_UID_VAR(buf, t, obj_script_id(obj), "object", 0); + ADD_UID_VAR(buf, t, object_id, "object", 0); ret_val = script_driver(&room, t, WLD_TRIGGER, TRIG_NEW); - if (obj->carried_by != actor) + // check for purged object before dereferencing. + if (!has_obj_by_uid_in_lookup_table(object_id) || obj->carried_by != actor) return 0; else return ret_val; From a60f0eefb8e5c705a0bc3c70c44ebddb6ce4f545 Mon Sep 17 00:00:00 2001 From: Thomas Arp <357770+welcor@users.noreply.github.com> Date: Sat, 7 Mar 2020 23:22:01 +0100 Subject: [PATCH 4/5] Minor bugfix in code that should be unreachable. We already log that we update, but no update was taking place. --- src/dg_scripts.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/dg_scripts.c b/src/dg_scripts.c index 709f5f5..2cf4539 100644 --- a/src/dg_scripts.c +++ b/src/dg_scripts.c @@ -2994,10 +2994,15 @@ void init_lookup_table(void) } } -static struct lookup_table_t *find_element_by_uid_in_lookup_table(long uid) +static struct lookup_table_t *get_bucket_head(long uid) { int bucket = (int) (uid & (BUCKET_COUNT - 1)); - struct lookup_table_t *lt = &lookup_table[bucket]; + return &lookup_table[bucket]; +} + +static struct lookup_table_t *find_element_by_uid_in_lookup_table(long uid) +{ + struct lookup_table_t *lt = get_bucket_head(uid); for (;lt && lt->uid != uid ; lt = lt->next) ; return lt; @@ -3034,8 +3039,7 @@ int has_obj_by_uid_in_lookup_table(long uid) void add_to_lookup_table(long uid, void *c) { - int bucket = (int) (uid & (BUCKET_COUNT - 1)); - struct lookup_table_t *lt = &lookup_table[bucket]; + struct lookup_table_t *lt = get_bucket_head(uid); if (lt && lt->uid == uid) { log("add_to_lookup updating existing value for uid=%ld (%p -> %p)", uid, lt->c, c); @@ -3046,6 +3050,7 @@ void add_to_lookup_table(long uid, void *c) for (;lt && lt->next; lt = lt->next) if (lt->next->uid == uid) { log("add_to_lookup updating existing value for uid=%ld (%p -> %p)", uid, lt->next->c, c); + lt->next->c = c; return; } From fc223452e836799ea88e5eefc538e29fba045ddb Mon Sep 17 00:00:00 2001 From: Thomas Arp <357770+welcor@users.noreply.github.com> Date: Sat, 7 Mar 2020 23:24:32 +0100 Subject: [PATCH 5/5] Fix typo in previous commit. Also, inline lookup functions if possible. --- src/dg_scripts.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dg_scripts.c b/src/dg_scripts.c index 2cf4539..978cf6d 100644 --- a/src/dg_scripts.c +++ b/src/dg_scripts.c @@ -2994,13 +2994,13 @@ void init_lookup_table(void) } } -static struct lookup_table_t *get_bucket_head(long uid) +static inline struct lookup_table_t *get_bucket_head(long uid) { int bucket = (int) (uid & (BUCKET_COUNT - 1)); return &lookup_table[bucket]; } -static struct lookup_table_t *find_element_by_uid_in_lookup_table(long uid) +static inline struct lookup_table_t *find_element_by_uid_in_lookup_table(long uid) { struct lookup_table_t *lt = get_bucket_head(uid);