From 90d515b6ab6edb0f0945bd932ed2192357b4164e Mon Sep 17 00:00:00 2001 From: PatR Date: Wed, 24 Jan 2024 14:31:26 -0800 Subject: [PATCH] fix #K4091 - losexp() assert(u.ulevel < 30) fail Previously reported via github pull request #1188 as an out of bounds access to u.uhpinc[], followed by issue #1189 when it was closed, the backtrace accompanying new assertion failure provided more information that led to figuring out the problem. Only mattered for the debug fuzzer; wouldn't happen in regular play. When the hero dies during fuzzing, the fuzzer sometimes restores lost levels via blessed potion of restore ability. If that happened to a hero who died by being life-drained while at level 1 then losexp()'s assumption that life-saved hero was still level 1 got violated. If levels had been lost all the way down from a peak of 30, restoration to u.ulevel==30 resulted in invalid array indexing into u.uhpinc[], then failure of 'assert(u.ulevel >= 0 && u.ulevel < MAXULEV)' which was added to avoid that. Pull request #1188 and issue #1189 are already closed, but they hadn't actually been solved yet. Fixes #1188 Fixes #1189 --- doc/fixes3-7-0.txt | 4 ++++ src/exper.c | 12 ++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/doc/fixes3-7-0.txt b/doc/fixes3-7-0.txt index ad61c7740..9e58d6ac7 100644 --- a/doc/fixes3-7-0.txt +++ b/doc/fixes3-7-0.txt @@ -1847,6 +1847,10 @@ stacked pair of potions of healing in supply chest weighed same as one potion; immediately followed a decrease message when the potion got used up bounds checking for object name formatting inadvertently introduced a change that hid BUC prefix for charged rings +debug fuzzer was triggering out of bounds array access in loseexp() if + life-saving at level 1 used blessed restore ability to regain lost + levels and restored those all the way to level 30; introducing an + assert(u.ulevel < MAXULEV) changed bounds issue to assertion failure Fixes to 3.7.0-x Platform and/or Interface Problems Exposed Via git Repository diff --git a/src/exper.c b/src/exper.c index 7ce1e418e..12ef1ba37 100644 --- a/src/exper.c +++ b/src/exper.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 exper.c $NHDT-Date: 1621380393 2021/05/18 23:26:33 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.46 $ */ +/* NetHack 3.7 exper.c $NHDT-Date: 1706133782 2024/01/24 22:03:02 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.62 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2007. */ /* NetHack may be freely redistributed. See license for details. */ @@ -222,13 +222,14 @@ losexp( in that situation */ if (u.ulevel > 1 || drainer) pline("%s level %d.", Goodbye(), u.ulevel); + if (u.ulevel > 1) { u.ulevel -= 1; /* remove intrinsic abilities */ adjabil(u.ulevel + 1, u.ulevel); livelog_printf(LL_MINORAC, "lost experience level %d", u.ulevel + 1); SoundAchievement(0, sa2_xpleveldown, 0); - } else { + } else { /* u.ulevel==1 */ if (drainer) { gk.killer.format = KILLED_BY; if (gk.killer.name != drainer) @@ -236,11 +237,14 @@ losexp( done(DIED); } /* no drainer or lifesaved */ + if (u.ulevel > 1) + /* can happen during debug fuzzing if fuzzer_savelife() uses + a blessed potion of restore ability to restore lost levels */ + return; u.uexp = 0; livelog_printf(LL_MINORAC, "lost all experience"); } - - assert(u.ulevel >= 0 && u.ulevel < MAXULEV); + assert(u.ulevel >= 0 && u.ulevel < MAXULEV); /* valid array index */ olduhpmax = u.uhpmax; uhpmin = minuhpmax(10); /* same minimum as is used by life-saving */