From 3cca4f27076e92a3be2ee9286813c18d8813311e Mon Sep 17 00:00:00 2001 From: nhmall Date: Sun, 26 Jan 2025 11:55:48 -0500 Subject: [PATCH] avoid mk_artifact()-related memory leaks Reported directly to devteam after being discovered in nerfhack. --- src/artifact.c | 47 +++++++++++++++++++++++++++++++---------------- src/mkobj.c | 21 +++++++++++++++------ src/mplayer.c | 1 + src/pray.c | 1 + 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/artifact.c b/src/artifact.c index 7b9c3864f..7b69820b8 100644 --- a/src/artifact.c +++ b/src/artifact.c @@ -35,6 +35,7 @@ staticfn uchar abil_to_adtyp(long *) NONNULLARG1; staticfn int glow_strength(int); staticfn boolean untouchable(struct obj *, boolean); staticfn int count_surround_traps(coordxy, coordxy); +staticfn void dispose_of_orig_obj(struct obj *); /* The amount added to the victim's total hit points to insure that the victim will be killed even after damage bonus/penalty adjustments. @@ -154,6 +155,7 @@ mk_artifact( uchar max_giftvalue, /* cap on generated giftvalue */ boolean adjust_spe) /* whether to add spe to situational artifacts */ { + struct obj *artiobj; const struct artifact *a; int m, n, altn; boolean by_align = (alignment != A_NONE); @@ -237,37 +239,50 @@ mk_artifact( m = eligible[rn2(n)]; /* [0..n-1] */ a = &artilist[m]; - /* make an appropriate object if necessary, then christen it */ - otmp = mksobj((int) a->otyp, TRUE, FALSE); + /* make an appropriate object, then christen it */ + artiobj = mksobj((int) a->otyp, TRUE, FALSE); if (adjust_spe) { int new_spe; - /* Adjust otmp->spe by a->gen_spe. (This is a no-op for + /* Adjust artiobj->spe by a->gen_spe. (This is a no-op for non-weapons, which always have a gen_spe of 0, and for many weapons, too.) The result is clamped into the "normal" range to prevent an outside chance of +12 artifacts generating. */ - new_spe = (int)otmp->spe + a->gen_spe; + new_spe = (int)artiobj->spe + a->gen_spe; if (new_spe >= -10 && new_spe < 10) - otmp->spe = new_spe; - } - - if (otmp) { - /* prevent erosion from generating */ - otmp->oeroded = otmp->oeroded2 = 0; - otmp = oname(otmp, a->name, ONAME_NO_FLAGS); - otmp->oartifact = m; - /* set existence and reason for creation bits */ - artifact_origin(otmp, ONAME_RANDOM); /* 'random' is default */ + artiobj->spe = new_spe; } + /* prevent erosion from generating */ + artiobj->oeroded = artiobj->oeroded2 = 0; + artiobj = oname(artiobj, a->name, ONAME_NO_FLAGS); + artiobj->oartifact = m; + /* set existence and reason for creation bits */ + artifact_origin(artiobj, ONAME_RANDOM); /* 'random' is default */ + if (otmp) + dispose_of_orig_obj(otmp); + otmp = artiobj; } else { /* nothing appropriate could be found; return original object */ - if (by_align) - otmp = 0; /* (there was no original object) */ + if (by_align && otmp) { + /* (there shouldn't have been an original object) */ + dispose_of_orig_obj(otmp); + otmp = 0; + } } return otmp; } +void +dispose_of_orig_obj(struct obj *obj) +{ + if (!obj) + return; + + obj_extract_self(obj); + obfree(obj, (struct obj *) 0); +} + /* * Returns the full name (with articles and correct capitalization) of an * artifact named "name" if one exists, or NULL, it not. diff --git a/src/mkobj.c b/src/mkobj.c index 2555386e3..d76e65f1b 100644 --- a/src/mkobj.c +++ b/src/mkobj.c @@ -9,7 +9,7 @@ staticfn boolean may_generate_eroded(struct obj *); staticfn void mkobj_erosions(struct obj *); staticfn void mkbox_cnts(struct obj *); staticfn unsigned nextoid(struct obj *, struct obj *); -staticfn void mksobj_init(struct obj *, boolean); +staticfn void mksobj_init(struct obj **, boolean); staticfn int item_on_ice(struct obj *); staticfn void shrinking_glob_gone(struct obj *); staticfn void obj_timer_checks(struct obj *, coordxy, coordxy, int); @@ -861,9 +861,10 @@ unknow_object(struct obj *obj) /* do some initialization to newly created object; otyp must already be set */ staticfn void -mksobj_init(struct obj *otmp, boolean artif) +mksobj_init(struct obj **obj, boolean artif) { int mndx, tryct; + struct obj *otmp = *obj; char let = objects[otmp->otyp].oc_class; switch (let) { @@ -880,8 +881,11 @@ mksobj_init(struct obj *otmp, boolean artif) if (is_poisonable(otmp) && !rn2(100)) otmp->opoisoned = 1; - if (artif && !rn2(20 + (10 * nartifact_exist()))) + if (artif && !rn2(20 + (10 * nartifact_exist()))) { + /* mk_artifact() with otmp and A_NONE will never return NULL */ otmp = mk_artifact(otmp, (aligntyp) A_NONE, 99, TRUE); + *obj = otmp; + } break; case FOOD_CLASS: otmp->oeaten = 0; @@ -1085,8 +1089,11 @@ mksobj_init(struct obj *otmp, boolean artif) otmp->spe = rne(3); } else blessorcurse(otmp, 10); - if (artif && !rn2(40 + (10 * nartifact_exist()))) + if (artif && !rn2(40 + (10 * nartifact_exist()))) { + /* mk_artifact() with otmp and A_NONE will never return NULL */ otmp = mk_artifact(otmp, (aligntyp) A_NONE, 99, TRUE); + *obj = otmp; + } /* simulate lacquered armor for samurai */ if (Role_if(PM_SAMURAI) && otmp->otyp == SPLINT_MAIL && (svm.moves <= 1 || In_quest(&u.uz))) { @@ -1176,7 +1183,7 @@ mksobj(int otyp, boolean init, boolean artif) otmp->pickup_prev = 0; if (init) - mksobj_init(otmp, artif); + mksobj_init(&otmp, artif); /* some things must get done (corpsenm, timers) even if init = 0 */ switch ((otmp->oclass == POTION_CLASS && otmp->otyp != POT_OIL) @@ -1231,8 +1238,10 @@ mksobj(int otyp, boolean init, boolean artif) } /* unique objects may have an associated artifact entry */ - if (objects[otyp].oc_unique && !otmp->oartifact) + if (objects[otyp].oc_unique && !otmp->oartifact) { + /* mk_artifact() with otmp and A_NONE will never return NULL */ otmp = mk_artifact(otmp, (aligntyp) A_NONE, 99, FALSE); + } otmp->owt = weight(otmp); return otmp; } diff --git a/src/mplayer.c b/src/mplayer.c index 9e839b8fd..70b94b0f5 100644 --- a/src/mplayer.c +++ b/src/mplayer.c @@ -261,6 +261,7 @@ mk_mplayer(struct permonst *ptr, coordxy x, coordxy y, boolean special) otmp->oerodeproof = 1; else if (!rn2(2)) otmp->greased = 1; + /* mk_artifact() with otmp and A_NONE will never return NULL */ if (special && rn2(2)) otmp = mk_artifact(otmp, A_NONE, 99, FALSE); /* usually increase stack size if stackable weapon */ diff --git a/src/pray.c b/src/pray.c index 827b03226..ba700c4dd 100644 --- a/src/pray.c +++ b/src/pray.c @@ -1793,6 +1793,7 @@ bestow_artifact(uchar max_giftvalue) if (do_bestow) { struct obj *otmp; + /* mk_artifact() with NULL obj and a_align() arg can return NULL */ otmp = mk_artifact((struct obj *) 0, a_align(u.ux, u.uy), max_giftvalue, TRUE); if (otmp) {