From 263e48c6f70fad9b75c90d6834ba7cb6e50ee086 Mon Sep 17 00:00:00 2001 From: Michael Meyer Date: Mon, 28 Nov 2022 15:42:19 -0500 Subject: [PATCH 1/2] Tell player when water damage removes grease This was totally silent, which -- at least for me -- has led to quite a few cases of believing my bag or cloak is still greased when it actually wore off the last time I took a dip. I think telling the player that the grease has worn off would be helpful, and is consistent with other types of water damage. The message is printed even if you are blind, since that seems to be true of all the other messages in water_damage(). I am not sure if that makes complete sense (especially for ones like a scroll fading -- some like water getting into a bag could be sensed by touch) but I didn't change anything there. --- src/trap.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/trap.c b/src/trap.c index b75b2a3d8..22ccb2af2 100644 --- a/src/trap.c +++ b/src/trap.c @@ -4146,10 +4146,13 @@ water_damage( wet_a_towel(obj, -rnd(7 - obj->spe), TRUE); return ER_NOTHING; } else if (obj->greased) { - if (!rn2(2)) + if (!rn2(2)) { obj->greased = 0; - if (carried(obj)) - update_inventory(); + if (carried(obj)) { + pline_The("grease on %s washes off.", yname(obj)); + update_inventory(); + } + } return ER_GREASED; } else if (Is_container(obj) && (!Waterproof_container(obj) || (obj->cursed && !rn2(3)))) { From 75ff2fa5fc06c1fde7a37cbafe40a01ea1034fa7 Mon Sep 17 00:00:00 2001 From: Michael Meyer Date: Mon, 28 Nov 2022 15:44:27 -0500 Subject: [PATCH 2/2] Fix: use-after-free when fountain dipping A potion of acid could be destroyed and freed by dipping into a fountain, then dereferenced after the fact -- both when checking its type immediately after the water_damage() call (as was noticed by hackemslashem and amateurhour on IRC), and also in the later switch/case a few lines further down in dipfountain(). I basically reversed the original 'er != ER_DESTROYED' test here: as it was before this, I think the only thing that could hit it was a greased potion of acid, which would survive the initial dip due to the grease. Such a potion would be silently deleted. Potions of acid which were actually destroyed by water_damage, on the other hand, could be allowed to continue down to the switch/case of further effects (and associated dereferences). I think this makes more sense in reverse, with potions that were protected by grease actually being protected and producing normal dip effects, and potions of acid which exploded causing an early return with no further effects. This effectively prevents the various use-after-free scenarios that were possible, too. --- src/fountain.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/fountain.c b/src/fountain.c index 13cc84ac9..6a066f059 100644 --- a/src/fountain.c +++ b/src/fountain.c @@ -429,12 +429,8 @@ dipfountain(register struct obj *obj) } else { er = water_damage(obj, NULL, TRUE); - if (obj->otyp == POT_ACID - && er != ER_DESTROYED) { /* Acid and water don't mix */ - useup(obj); - return; - } else if (er != ER_NOTHING && !rn2(2)) { /* no further effect */ - return; + if (er == ER_DESTROYED || (er != ER_NOTHING && !rn2(2))) { + return; /* no further effect */ } }