From 75ff2fa5fc06c1fde7a37cbafe40a01ea1034fa7 Mon Sep 17 00:00:00 2001 From: Michael Meyer Date: Mon, 28 Nov 2022 15:44:27 -0500 Subject: [PATCH] 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 */ } }