From 4ede5f1cd4097479e24e2eb9a6cdc4ac0ac0491b Mon Sep 17 00:00:00 2001 From: nhmall Date: Thu, 1 Dec 2022 03:48:11 -0500 Subject: [PATCH] Use-after-free with engulfer in xkilled #938 If you were on a level teleporter, the spoteffects() call after the hero gets expelled could end up going to a new level and freeing all the monst chains from the level you were originally engulfed on. #0 0xba0507 in free #1 0x87feda in dealloc_monst src/mon.c:2369 #2 0x880a02 in dmonsfree src/mon.c:2194 #3 0x9a7aa2 in savelev_core src/save.c:507 #4 0x9a7a21 in savelev src/save.c:466 #5 0x71eb9d in goto_level src/do.c:1483 #6 0x71833f in deferred_goto src/do.c:1903 #7 0xa2533f in level_tele src/teleport.c:1117 #8 0xa2567b in level_tele_trap src/teleport.c:1198 #9 0xa5c007 in trapeffect_level_telep src/trap.c:1861 #10 0xa5f856 in trapeffect_selector src/trap.c:2497 #11 0xa47497 in dotrap src/trap.c:2586 #12 0x7d669b in spoteffects src/hack.c:2859 #13 0x89d495 in xkilled src/mon.c:3187 The latter parts of xkilled() after the spoteffects() call would then attempt to dereference the free'd monst pointer. Save a copy of the monst struct prior to spoteffects() if you were expelled, then point at the reference copy afterwards. Resolves #938 --- src/mon.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/mon.c b/src/mon.c index b15eca277..c0c4a51b6 100644 --- a/src/mon.c +++ b/src/mon.c @@ -3044,6 +3044,7 @@ xkilled( { int tmp, mndx; coordxy x = mtmp->mx, y = mtmp->my; + struct monst museum = cg.zeromonst; struct permonst *mdat; struct obj *otmp; struct trap *t; @@ -3183,8 +3184,16 @@ xkilled( } } } - if (wasinside) + + if (wasinside) { + /* spoteffects() can end up clearing the level of monsters; grab a copy */ + museum = *mtmp; + museum.nmon = 0; + museum.minvent = 0; + museum.mextra = 0; spoteffects(TRUE); /* poor man's expels() */ + mtmp = &museum; /* use the reference copy now */ + } /* monster is gone, corpse or other object might now be visible */ newsym(x, y);