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.
This commit is contained in:
@@ -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 */
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user