Merge fix/wizard-which-bounds (bound wizard-created item kinds)
This commit was merged in pull request #20.
This commit is contained in:
43
TODO.md
43
TODO.md
@@ -34,6 +34,49 @@ wizard commands).
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-09 Wizard-create bounds fix (`fix/wizard-which-bounds`, closes #10):
|
||||
`createObj` stored the raw `0-f` nibble as `Object.Which` with no bounds
|
||||
check, so wizard mode -> `C` -> `/` -> `f` produced a wand numbered 15 against
|
||||
a 14-entry table and panicked in `fixStick`. Input outside `0-f` overshoots
|
||||
much further rather than going negative: `readchar` returns a `byte`, so the
|
||||
`int(ch-'a') + 10` branch is byte arithmetic and wraps — `'A'` gives 234 and
|
||||
`'!'` gives 202 — and panicked the same way. C's `create_obj()` was equally
|
||||
unchecked, but every C consumer was either a `switch` (defined for any value)
|
||||
or a static-array read past the end (undefined, and survivable in practice),
|
||||
whereas since refactor step 8 one game is one process, so the Go panic kills
|
||||
the game with the terminal still in raw mode. Fixed at the two boundaries a
|
||||
bad `Which` can enter through: `createObj` now rejects an out-of-range choice
|
||||
with a message built from C's own `type_name()` vocabulary and adds nothing to
|
||||
the pack (a deliberate, commented divergence, since C had no defined behavior
|
||||
here to be faithful to), and `Restore` refuses a snapshot describing such an
|
||||
object (`ErrSaveCorrupt`) instead of loading a game that would explode later.
|
||||
Behind those, `whichLimit`/`hasValidWhich` back defensive guards at every
|
||||
dispatch named in the issue: the three effect tables (the new `quaffHandler`,
|
||||
`readHandler`, and `zapHandler` accessors return no handler rather than
|
||||
indexing — for wands that is exactly non-`MASTER` C, which matched no case and
|
||||
still ran `o_charges--`), the `callIt` lore lookups, `identifyType` (whose
|
||||
table is shorter than the scroll table keying it, though no scroll that can
|
||||
reach `readIdentify` overshoots it, so that one is defensive rather than a
|
||||
live bound), `armorClass` for the four `a_class[]` reads, `initWeapon` against
|
||||
the missing `init_dam[]` row for `WeaponFlame`, `fixStick`'s `ws_type[]` read,
|
||||
and `inventoryName`, hoisted so one check covers the scroll-title read the
|
||||
issue listed plus its potion-color, ring-stone, wand-material, weapon and
|
||||
armor siblings. `objectWorth` got the same hoisted guard, since the
|
||||
death-screen appraisal reads the identical per-kind tables. No in-range input
|
||||
changes behavior and no guard consumes a random number — the rejection
|
||||
precedes every `rnd()` call, verified both by an explicit seed-unchanged test
|
||||
and by `TestSeedCompatItemTables` staying green untouched. New
|
||||
`game/wizard_test.go`: the exact reproducer, a rejection sweep over every
|
||||
indexed kind including both wrapping-input forms, an acceptance sweep proving
|
||||
valid choices still build the right item, one no-panic test per guarded family
|
||||
(wand/potion/scroll/armor/weapon), the `fixStick` crash site, the corrupt-save
|
||||
rejection over the wrapped values and a negative `Which` (a decoded snapshot
|
||||
is the only source of one, so it is what exercises the `Which >= 0` arm of
|
||||
`hasValidWhich`), and a check that `whichLimit` still agrees with the table
|
||||
sizes. Each guard was confirmed load-bearing by reverting it and watching the
|
||||
test panic. `Next Step` deliberately not rotated: this was out-of-band issue
|
||||
work.
|
||||
|
||||
- 2026-08-09 Stale-docs correction (`docs-staleness`, closes #3): four claims in
|
||||
`MEMORY.md`/`TODO.md`/`README.md` had gone false and were misdirecting agents
|
||||
— the reviewer on PR #9 repeated one of them verbatim. Each was re-verified
|
||||
|
||||
@@ -164,6 +164,38 @@ func newObject() *Object {
|
||||
return &Object{Launch: noWeapon}
|
||||
}
|
||||
|
||||
// whichLimit reports how many entries a kind's per-kind tables hold, so
|
||||
// Which is a legal index exactly while 0 <= Which < whichLimit(Kind).
|
||||
// Kinds whose Which is not a table index at all — food (0 ration, 1 the
|
||||
// fruit), the amulet, gold, and the KindNone an unrecognized glyph maps
|
||||
// to — report 0, meaning any value is acceptable, which is what C did
|
||||
// with them too.
|
||||
//
|
||||
// Weapons count one past NumWeaponTypes on purpose: Items.Weapons is
|
||||
// sized NumWeaponTypes+1 for the WeaponFlame dragon-breath entry, and
|
||||
// fireBolt really does set Which = WeaponFlame on a live Object. That
|
||||
// entry has no init_dam[] row, so initWeapon and createObj bound
|
||||
// themselves by NumWeaponTypes instead.
|
||||
func whichLimit(kind ObjectKind) int {
|
||||
//nolint:exhaustive // the remaining kinds do not index a per-kind table
|
||||
switch kind {
|
||||
case KindPotion:
|
||||
return int(NumPotionTypes)
|
||||
case KindScroll:
|
||||
return int(NumScrollTypes)
|
||||
case KindRing:
|
||||
return int(NumRingTypes)
|
||||
case KindWand:
|
||||
return int(NumWandTypes)
|
||||
case KindArmor:
|
||||
return int(NumArmorTypes)
|
||||
case KindWeapon:
|
||||
return int(NumWeaponTypes) + 1
|
||||
}
|
||||
|
||||
return 0
|
||||
}
|
||||
|
||||
// PotionKind returns Which as a potion kind; valid only for KindPotion.
|
||||
func (o *Object) PotionKind() PotionKind { return PotionKind(o.Which) }
|
||||
|
||||
@@ -182,6 +214,36 @@ func (o *Object) WeaponKind() WeaponKind { return WeaponKind(o.Which) }
|
||||
// ArmorKind returns Which as an armor kind; valid only for KindArmor.
|
||||
func (o *Object) ArmorKind() ArmorKind { return ArmorKind(o.Which) }
|
||||
|
||||
// hasValidWhich reports whether Which is a legal index into this
|
||||
// object's per-kind tables. Objects the game builds itself always
|
||||
// satisfy it; the wizard-create command and a restored save file are the
|
||||
// only two ways a malformed one can appear, and both reject it (see
|
||||
// createObj and validateSnapshotObjects).
|
||||
//
|
||||
// The Which >= 0 arm is unreachable from the keyboard: createObj derives
|
||||
// Which with byte arithmetic (int(ch-'a') + 10), which wraps to a large
|
||||
// positive value rather than going negative. It is kept as
|
||||
// defense-in-depth for the non-keyboard source — a decoded save file,
|
||||
// where Which is an int off the wire and can hold anything — and is
|
||||
// exercised there by TestRestoreRejectsOutOfRangeWhich.
|
||||
func (o *Object) hasValidWhich() bool {
|
||||
limit := whichLimit(o.Kind)
|
||||
|
||||
return limit == 0 || (o.Which >= 0 && o.Which < limit)
|
||||
}
|
||||
|
||||
// wizardCanCreate reports whether Which names something the wizard-create
|
||||
// command can actually build. It is hasValidWhich narrowed for weapons:
|
||||
// WeaponFlame owns a name-table slot but no init_dam[] row, so asking for
|
||||
// it would leave initWeapon nothing to copy.
|
||||
func (o *Object) wizardCanCreate() bool {
|
||||
if o.Kind == KindWeapon {
|
||||
return o.Which >= 0 && o.Which < int(NumWeaponTypes)
|
||||
}
|
||||
|
||||
return o.hasValidWhich()
|
||||
}
|
||||
|
||||
// attachObj is list.c attach(): push item onto the front of a list.
|
||||
func attachObj(list *[]*Object, item *Object) {
|
||||
*list = append([]*Object{item}, *list...)
|
||||
|
||||
@@ -42,13 +42,16 @@ func (g *RogueGame) quaff() {
|
||||
|
||||
g.leavePack(obj, false, false)
|
||||
|
||||
if h := g.data.quaffHandlers[obj.PotionKind()]; h != nil {
|
||||
if h := g.data.quaffHandler(obj); h != nil {
|
||||
h(g, trip)
|
||||
}
|
||||
|
||||
g.status()
|
||||
// Throw the item away
|
||||
g.callIt(&g.Items.Potions[obj.Which])
|
||||
// Throw the item away. A malformed potion has no lore entry to name,
|
||||
// so it is drunk for no effect and never prompts to be called anything.
|
||||
if obj.hasValidWhich() {
|
||||
g.callIt(&g.Items.Potions[obj.Which])
|
||||
}
|
||||
}
|
||||
|
||||
// The per-potion effect handlers, dispatched through
|
||||
@@ -265,7 +268,7 @@ func (g *RogueGame) isMagic(o *Object) bool {
|
||||
//nolint:exhaustive // C-faithful: only the cases C handled (approved 2026-07-07)
|
||||
switch o.Kind {
|
||||
case KindArmor:
|
||||
return o.Flags.Has(Protected) || o.ArmorClass != g.data.aClass[o.Which]
|
||||
return o.Flags.Has(Protected) || o.ArmorClass != g.data.armorClass(o.Which)
|
||||
case KindWeapon:
|
||||
return o.HPlus != 0 || o.DPlus != 0
|
||||
case KindPotion, KindScroll, KindWand, KindRing, KindAmulet:
|
||||
|
||||
@@ -132,6 +132,13 @@ func (g *RogueGame) totalWinner() {
|
||||
// objectWorth appraises one pack item on the way out, marking it known
|
||||
// (the switch of rip.c total_winner).
|
||||
func (g *RogueGame) objectWorth(obj *Object) int {
|
||||
// Same defense as inventoryName: most arms below appraise from a
|
||||
// per-kind table at obj.Which, so a malformed item is worth nothing
|
||||
// rather than a panic on the way to the scoreboard.
|
||||
if !obj.hasValidWhich() {
|
||||
return 0
|
||||
}
|
||||
|
||||
it := &g.Items
|
||||
worth := 0
|
||||
|
||||
@@ -146,7 +153,7 @@ func (g *RogueGame) objectWorth(obj *Object) int {
|
||||
case KindArmor:
|
||||
worth = it.Armors[obj.Which].Worth
|
||||
worth += (9 - obj.ArmorClass) * 100
|
||||
worth += 10 * (g.data.aClass[obj.Which] - obj.ArmorClass)
|
||||
worth += 10 * (g.data.armorClass(obj.Which) - obj.ArmorClass)
|
||||
obj.Flags.Set(Known)
|
||||
case KindScroll:
|
||||
worth = loreWorth(&it.Scrolls[obj.Which], obj.Count)
|
||||
|
||||
36
game/save.go
36
game/save.go
@@ -680,6 +680,37 @@ func (g *RogueGame) AutoSave() {
|
||||
// ErrSaveOutOfDate reports a save file from an incompatible version.
|
||||
var ErrSaveOutOfDate = errors.New("sorry, saved game is out of date")
|
||||
|
||||
// ErrSaveCorrupt reports a save file whose contents are structurally
|
||||
// impossible for a game this code could have written.
|
||||
var ErrSaveCorrupt = errors.New("sorry, saved game is corrupt")
|
||||
|
||||
// validateSnapshotObjects rejects a snapshot carrying an item whose Which
|
||||
// would index past the end of its kind's tables. The file is written by
|
||||
// this program, so such a value can only come from corruption or
|
||||
// tampering; refusing it at the door is what keeps a malformed object
|
||||
// from reaching the effect tables in doZap, quaff, and readScroll, where
|
||||
// the wizard-create bug (issue #10) used to put one.
|
||||
func validateSnapshotObjects(st *SaveState) error {
|
||||
lists := make([][]Object, 0, 2+len(st.Monsters))
|
||||
lists = append(lists, st.Objects, st.Player.Body.Pack)
|
||||
|
||||
for i := range st.Monsters {
|
||||
lists = append(lists, st.Monsters[i].Pack)
|
||||
}
|
||||
|
||||
for _, list := range lists {
|
||||
for i := range list {
|
||||
obj := &list[i]
|
||||
if !obj.hasValidWhich() {
|
||||
return fmt.Errorf("%w: %s has out-of-range which %d",
|
||||
ErrSaveCorrupt, obj.Kind, obj.Which)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Restore restores a saved game from a file (save.c restore). The file is
|
||||
// deleted, as in C, to defeat restarting from the same save.
|
||||
func Restore(path string, params Params) (*RogueGame, error) {
|
||||
@@ -702,6 +733,11 @@ func Restore(path string, params Params) (*RogueGame, error) {
|
||||
return nil, ErrSaveOutOfDate
|
||||
}
|
||||
|
||||
valErr := validateSnapshotObjects(&st)
|
||||
if valErr != nil {
|
||||
return nil, valErr
|
||||
}
|
||||
|
||||
g := &RogueGame{
|
||||
data: newGameData(),
|
||||
Rng: &Rng{},
|
||||
|
||||
@@ -29,14 +29,17 @@ func (g *RogueGame) readScroll() {
|
||||
// Get rid of the thing
|
||||
g.leavePack(obj, false, false)
|
||||
|
||||
if h := g.data.readHandlers[obj.ScrollKind()]; h != nil {
|
||||
if h := g.data.readHandler(obj); h != nil {
|
||||
h(g, obj)
|
||||
}
|
||||
|
||||
g.look(true) // put the result of the scroll on the screen
|
||||
g.status()
|
||||
|
||||
g.callIt(&g.Items.Scrolls[obj.Which])
|
||||
// A malformed scroll has no lore entry to name (see quaff).
|
||||
if obj.hasValidWhich() {
|
||||
g.callIt(&g.Items.Scrolls[obj.Which])
|
||||
}
|
||||
}
|
||||
|
||||
// The per-scroll effect handlers, dispatched through
|
||||
@@ -166,10 +169,15 @@ func (g *RogueGame) createMonsterSpot() (Coord, bool) {
|
||||
}
|
||||
|
||||
func (g *RogueGame) readIdentify(obj *Object) {
|
||||
// Identify, let him figure something out
|
||||
// Identify, let him figure something out. idType is shorter than the
|
||||
// scroll table keying it (it stops after the last identify scroll),
|
||||
// so the filter lookup carries its own bound. That bound is not
|
||||
// reachable today: readHandlers registers readIdentify only for the
|
||||
// identify scrolls, all of which sit inside idType. It is kept as
|
||||
// defense-in-depth against a future table resize.
|
||||
g.Items.Scrolls[obj.Which].Know = true
|
||||
g.msg("this scroll is an %s scroll", g.Items.Scrolls[obj.Which].Name)
|
||||
g.whatis(true, g.data.idType[obj.ScrollKind()])
|
||||
g.whatis(true, g.data.identifyType(obj.ScrollKind()))
|
||||
}
|
||||
|
||||
func (g *RogueGame) readMagicMapping(*Object) {
|
||||
|
||||
@@ -31,7 +31,11 @@ func (g *RogueGame) doZap() {
|
||||
return
|
||||
}
|
||||
|
||||
if h := g.data.zapHandlers[obj.WandKind()]; h != nil {
|
||||
// A malformed Which yields no handler, which is exactly what C did in a
|
||||
// non-MASTER build: its do_zap switch matched no case, fell out, and
|
||||
// still ran o_charges--. (The MASTER-only "what a bizarre schtick!"
|
||||
// message for that arm is issue #13, not this bounds fix.)
|
||||
if h := g.data.zapHandler(obj); h != nil {
|
||||
if !h(g, obj) {
|
||||
return // the zap aborted; no charge is used
|
||||
}
|
||||
@@ -502,7 +506,11 @@ func (g *RogueGame) boltStrikesHero(start Coord, name string, fromHero bool) boo
|
||||
|
||||
// fixStick sets up a new wand or staff (sticks.c fix_stick).
|
||||
func (g *RogueGame) fixStick(cur *Object) {
|
||||
if g.Items.WandType[cur.Which] == staffName {
|
||||
// ws_type[] is indexed by Which; a malformed one is treated as a wand,
|
||||
// which is the branch the C string compare would take against any
|
||||
// value that is not literally "staff". The charge switch below already
|
||||
// funnels everything but WandLight into its default arm.
|
||||
if cur.hasValidWhich() && g.Items.WandType[cur.Which] == staffName {
|
||||
cur.Damage = dice("2x3")
|
||||
} else {
|
||||
cur.Damage = dice("1x1")
|
||||
|
||||
@@ -859,6 +859,69 @@ func newGameData() *gameData {
|
||||
}
|
||||
}
|
||||
|
||||
// The three effect-table lookups below are the guarded form of a raw
|
||||
// index into quaffHandlers/readHandlers/zapHandlers. An object whose
|
||||
// Which is out of range for its kind reports no handler rather than
|
||||
// panicking, which lands on the same do-nothing behavior C's switches
|
||||
// had when no case matched.
|
||||
|
||||
// quaffHandler returns the potion effect for obj, or nil when obj is not
|
||||
// a potion the table knows about.
|
||||
func (d *gameData) quaffHandler(obj *Object) func(g *RogueGame, trip bool) {
|
||||
if !obj.hasValidWhich() {
|
||||
return nil
|
||||
}
|
||||
|
||||
return d.quaffHandlers[obj.PotionKind()]
|
||||
}
|
||||
|
||||
// readHandler returns the scroll effect for obj, or nil when obj is not
|
||||
// a scroll the table knows about.
|
||||
func (d *gameData) readHandler(obj *Object) func(g *RogueGame, obj *Object) {
|
||||
if !obj.hasValidWhich() {
|
||||
return nil
|
||||
}
|
||||
|
||||
return d.readHandlers[obj.ScrollKind()]
|
||||
}
|
||||
|
||||
// zapHandler returns the wand effect for obj, or nil when obj is not a
|
||||
// wand the table knows about.
|
||||
func (d *gameData) zapHandler(obj *Object) func(g *RogueGame, obj *Object) bool {
|
||||
if !obj.hasValidWhich() {
|
||||
return nil
|
||||
}
|
||||
|
||||
return d.zapHandlers[obj.WandKind()]
|
||||
}
|
||||
|
||||
// identifyType reads extern.c's identify-scroll to item-kind map,
|
||||
// returning KindNone for any scroll past the last identify scroll (the
|
||||
// table is shorter than the scroll table it is keyed by). No scroll that
|
||||
// can reach readIdentify is past that end, so the guard is defensive
|
||||
// rather than a live bound; it exists so a future table resize cannot
|
||||
// turn the lookup into a panic.
|
||||
func (d *gameData) identifyType(kind ScrollKind) ObjectKind {
|
||||
if kind < 0 || int(kind) >= len(d.idType) {
|
||||
return KindNone
|
||||
}
|
||||
|
||||
return d.idType[kind]
|
||||
}
|
||||
|
||||
// armorClass reads extern.c a_class[] for an armor object's Which,
|
||||
// returning 0 when Which is out of range. Only a malformed object can
|
||||
// hit that arm — createObj and Restore both reject one — but a bounds
|
||||
// check here is what keeps a bad index from panicking the process
|
||||
// instead of merely naming a suit of armor oddly.
|
||||
func (d *gameData) armorClass(which int) int {
|
||||
if which < 0 || which >= int(NumArmorTypes) {
|
||||
return 0
|
||||
}
|
||||
|
||||
return d.aClass[which]
|
||||
}
|
||||
|
||||
// Version strings (vers.c). The encstr/statlist XOR keys are not ported:
|
||||
// the Go save format does not use them.
|
||||
const (
|
||||
|
||||
@@ -13,6 +13,16 @@ import (
|
||||
func (g *RogueGame) inventoryName(obj *Object, drop bool) string {
|
||||
var pb strings.Builder
|
||||
|
||||
// Every arm below reaches into a per-kind name table at obj.Which:
|
||||
// the potion colors, ring stones, wand material/type, scroll titles,
|
||||
// and the weapon and armor name tables. An object with an out-of-range
|
||||
// Which cannot reach here (createObj and Restore both reject one), but
|
||||
// if one ever did, naming it must not take the process down — so it
|
||||
// falls back to the bare category name from C's type_name() vocabulary.
|
||||
if !obj.hasValidWhich() {
|
||||
return obj.Kind.String()
|
||||
}
|
||||
|
||||
which := obj.Which
|
||||
it := &g.Items
|
||||
|
||||
@@ -112,7 +122,7 @@ func (g *RogueGame) nameArmor(pb *strings.Builder, obj *Object) {
|
||||
sp := g.Items.Armors[obj.Which].Name
|
||||
if obj.Flags.Has(Known) {
|
||||
fmt.Fprintf(pb, "%s %s [",
|
||||
num(g.data.aClass[obj.Which]-obj.ArmorClass, 0, Armor), sp)
|
||||
num(g.data.armorClass(obj.Which)-obj.ArmorClass, 0, Armor), sp)
|
||||
|
||||
if !g.Options.Terse {
|
||||
pb.WriteString("protection ")
|
||||
|
||||
@@ -159,6 +159,15 @@ type weaponSetup struct {
|
||||
|
||||
// initWeapon sets up a new weapon (weapons.c init_weapon).
|
||||
func (g *RogueGame) initWeapon(weap *Object, which WeaponKind) {
|
||||
// init_dam[] has a row only for the real weapons: WeaponFlame (dragon
|
||||
// breath) and anything past it have none. createObj rejects such a
|
||||
// choice before calling here, so this arm is unreachable in practice;
|
||||
// it exists so a malformed kind leaves the weapon untouched instead of
|
||||
// panicking on the table read.
|
||||
if which < 0 || int(which) >= int(NumWeaponTypes) {
|
||||
return
|
||||
}
|
||||
|
||||
iwp := &g.data.initWeaps[which]
|
||||
weap.Kind = KindWeapon
|
||||
weap.Which = int(which)
|
||||
|
||||
@@ -26,6 +26,23 @@ func (g *RogueGame) createObj() {
|
||||
obj.Count = 1
|
||||
g.Msgs.Mpos = 0
|
||||
|
||||
// Deliberate divergence from 5.4.4: C stored this nibble unchecked, so
|
||||
// 'a'-'f' indexed straight past the ends of the per-kind static
|
||||
// tables. Input outside '0'-'9' and 'a'-'f' overshoots much further:
|
||||
// readchar returns a byte, so ch-'a' above is byte arithmetic and
|
||||
// wraps instead of going negative ('A' gives 234, '!' gives 202).
|
||||
// Reading past a static array was undefined behavior C happened to
|
||||
// survive by picking up adjacent memory; in Go it is a panic that
|
||||
// kills the process with the terminal still in raw mode. C had no
|
||||
// defined behavior here to be faithful to, so the choice is rejected
|
||||
// outright rather than emulating a garbage read. The check precedes
|
||||
// every rnd() call below, so the RNG sequence is untouched either way.
|
||||
if !obj.wizardCanCreate() {
|
||||
g.msg("there is no such %s", obj.Kind)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
//nolint:exhaustive // C-faithful: only the cases C handled (approved 2026-07-07)
|
||||
switch obj.Kind {
|
||||
case KindWeapon, KindArmor:
|
||||
@@ -71,7 +88,7 @@ func (g *RogueGame) createWeaponArmor(obj *Object) {
|
||||
return
|
||||
}
|
||||
|
||||
obj.ArmorClass = g.data.aClass[obj.Which]
|
||||
obj.ArmorClass = g.data.armorClass(obj.Which)
|
||||
if bless == '-' {
|
||||
obj.ArmorClass += g.rnd(3) + 1
|
||||
}
|
||||
|
||||
426
game/wizard_test.go
Normal file
426
game/wizard_test.go
Normal file
@@ -0,0 +1,426 @@
|
||||
//nolint:testpackage // white-box tests reach unexported state (approved 2026-07-07)
|
||||
package game
|
||||
|
||||
import (
|
||||
"encoding/gob"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// mustNotPanic runs fn and turns a panic into an ordinary test failure.
|
||||
// The bug these tests cover (issue #10) panicked out of an array index,
|
||||
// and an unrecovered panic would take the whole test binary down instead
|
||||
// of reporting which dispatch regressed.
|
||||
func mustNotPanic(t *testing.T, what string, fn func()) {
|
||||
t.Helper()
|
||||
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
t.Errorf("%s panicked: %v", what, r)
|
||||
}
|
||||
}()
|
||||
|
||||
fn()
|
||||
}
|
||||
|
||||
// TestCreateObjWandFReproducer is the exact reported crash: wizard mode,
|
||||
// C, '/' for a wand, 'f' for which. 'f' is nibble 15 and there are only
|
||||
// NumWandTypes (14) wands, so fixStick used to index two past the end of
|
||||
// ws_type[] and panic.
|
||||
func TestCreateObjWandFReproducer(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
g.Wizard = true
|
||||
before := len(g.Player.Pack)
|
||||
|
||||
setInput(t, g, '/', 'f')
|
||||
|
||||
mustNotPanic(t, "createObj with wand 'f'", g.createObj)
|
||||
|
||||
if len(g.Player.Pack) != before {
|
||||
t.Errorf("out-of-range wand was added to the pack: %d items, want %d",
|
||||
len(g.Player.Pack), before)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCreateObjRejectsOutOfRangeWhich sweeps the rejection across every
|
||||
// kind whose Which is a table index, including input outside '0'-'f'.
|
||||
// isDigit is false for such input, so it takes the letter branch, where
|
||||
// ch-'a' is byte arithmetic and wraps rather than going negative: 'A'
|
||||
// (65) gives int(224)+10 == 234 and '!' (33) gives int(192)+10 == 202.
|
||||
// Those far-past-the-end values, not negative ones, are what the guard
|
||||
// has to catch on the keyboard path.
|
||||
func TestCreateObjRejectsOutOfRangeWhich(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
typ byte
|
||||
which byte
|
||||
}{
|
||||
{"wand f is past NumWandTypes", Stick, 'f'},
|
||||
{"potion f is past NumPotionTypes", Potion, 'f'},
|
||||
{"ring f is past NumRingTypes", Ring, 'f'},
|
||||
// NumScrollTypes is 18, past the 'f' the prompt tops out at, so a
|
||||
// scroll can only be driven out of range by input that wraps.
|
||||
{"scroll 'A' wraps to 234", Scroll, 'A'},
|
||||
{"armor 9 is past NumArmorTypes", Armor, '9'},
|
||||
{"weapon 9 is the flame pseudo-weapon", Weapon, '9'},
|
||||
{"wand 'A' wraps to 234", Stick, 'A'},
|
||||
{"wand '!' wraps to 202", Stick, '!'},
|
||||
{"armor 'A' wraps to 234", Armor, 'A'},
|
||||
{"weapon '!' wraps to 202", Weapon, '!'},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
g.Wizard = true
|
||||
before := len(g.Player.Pack)
|
||||
|
||||
setInput(t, g, tc.typ, tc.which)
|
||||
|
||||
mustNotPanic(t, tc.name, g.createObj)
|
||||
|
||||
if len(g.Player.Pack) != before {
|
||||
t.Errorf("pack grew to %d items, want %d: a rejected item was created",
|
||||
len(g.Player.Pack), before)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestCreateObjAcceptsValidWhich pins the other half of the contract: the
|
||||
// bounds check must not touch any in-range choice.
|
||||
func TestCreateObjAcceptsValidWhich(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
typ byte
|
||||
which byte
|
||||
kind ObjectKind
|
||||
want int
|
||||
}{
|
||||
{"wand of light", Stick, '0', KindWand, int(WandLight)},
|
||||
{"potion 0", Potion, '0', KindPotion, int(PotionConfusion)},
|
||||
{"scroll 9", Scroll, '9', KindScroll, int(ScrollIdentifyRingOrStick)},
|
||||
{"ring d, the last ring", Ring, 'd', KindRing, int(NumRingTypes) - 1},
|
||||
{"armor 7, the last armor", Armor, '7', KindArmor, int(NumArmorTypes) - 1},
|
||||
{"weapon 8, the last real weapon", Weapon, '8', KindWeapon, int(WeaponSpear)},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
g.Wizard = true
|
||||
|
||||
before := make(map[*Object]bool, len(g.Player.Pack))
|
||||
for _, o := range g.Player.Pack {
|
||||
before[o] = true
|
||||
}
|
||||
|
||||
// The armor and weapon arms read one more character for the
|
||||
// blessing prompt; 'n' means neither cursed nor blessed.
|
||||
setInput(t, g, tc.typ, tc.which, 'n')
|
||||
g.createObj()
|
||||
|
||||
if len(g.Player.Pack) != len(before)+1 {
|
||||
t.Fatalf("pack has %d items, want %d: valid item not created",
|
||||
len(g.Player.Pack), len(before)+1)
|
||||
}
|
||||
|
||||
// addPack files the new item in kind order, so find it by
|
||||
// identity rather than assuming it landed at the end.
|
||||
var made *Object
|
||||
|
||||
for _, o := range g.Player.Pack {
|
||||
if !before[o] {
|
||||
made = o
|
||||
}
|
||||
}
|
||||
|
||||
if made.Kind != tc.kind || made.Which != tc.want {
|
||||
t.Errorf("created %v which %d, want %v which %d",
|
||||
made.Kind, made.Which, tc.kind, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestCreateObjKeepsRNGSequence proves the guard costs no RNG draws: a
|
||||
// rejected creation must leave the generator exactly where it was, or
|
||||
// every later roll in the game would shift.
|
||||
func TestCreateObjKeepsRNGSequence(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
g.Wizard = true
|
||||
before := g.Rng.Seed
|
||||
|
||||
setInput(t, g, Stick, 'f')
|
||||
g.createObj()
|
||||
|
||||
if g.Rng.Seed != before {
|
||||
t.Errorf("rejected creation consumed RNG: seed %d, want %d",
|
||||
g.Rng.Seed, before)
|
||||
}
|
||||
}
|
||||
|
||||
// malformed builds an object of the given kind whose Which sits one past
|
||||
// the end of that kind's table — the state the wizard bug used to leave
|
||||
// behind, and the state a corrupt save file could still describe.
|
||||
func malformed(kind ObjectKind) *Object {
|
||||
obj := newObject()
|
||||
obj.Kind = kind
|
||||
obj.Which = whichLimit(kind)
|
||||
obj.Count = 1
|
||||
|
||||
return obj
|
||||
}
|
||||
|
||||
// TestZapMalformedWandDoesNotPanic covers the sticks.go dispatch. C's
|
||||
// non-MASTER do_zap matched no case and still ran o_charges--, so the
|
||||
// charge must be spent even though nothing happens.
|
||||
func TestZapMalformedWandDoesNotPanic(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
wand := malformed(KindWand)
|
||||
wand.Charges = 3
|
||||
ch := give(g, wand)
|
||||
|
||||
setInput(t, g, ch)
|
||||
|
||||
mustNotPanic(t, "doZap on a malformed wand", g.doZap)
|
||||
|
||||
if wand.Charges != 2 {
|
||||
t.Errorf("charges = %d after zapping, want 2", wand.Charges)
|
||||
}
|
||||
}
|
||||
|
||||
// TestQuaffMalformedPotionDoesNotPanic covers the potions.go dispatch and
|
||||
// the callIt lookup that follows it.
|
||||
func TestQuaffMalformedPotionDoesNotPanic(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
before := len(g.Player.Pack)
|
||||
ch := give(g, malformed(KindPotion))
|
||||
|
||||
setInput(t, g, ch)
|
||||
|
||||
mustNotPanic(t, "quaff of a malformed potion", g.quaff)
|
||||
|
||||
if len(g.Player.Pack) != before {
|
||||
t.Errorf("pack has %d items, want %d: the potion was not consumed",
|
||||
len(g.Player.Pack), before)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReadMalformedScrollDoesNotPanic covers the scrolls.go dispatch and
|
||||
// its callIt lookup.
|
||||
func TestReadMalformedScrollDoesNotPanic(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
before := len(g.Player.Pack)
|
||||
ch := give(g, malformed(KindScroll))
|
||||
|
||||
setInput(t, g, ch)
|
||||
|
||||
mustNotPanic(t, "readScroll of a malformed scroll", g.readScroll)
|
||||
|
||||
if len(g.Player.Pack) != before {
|
||||
t.Errorf("pack has %d items, want %d: the scroll was not consumed",
|
||||
len(g.Player.Pack), before)
|
||||
}
|
||||
}
|
||||
|
||||
// TestMalformedArmorDoesNotPanic covers the a_class[] reads: pricing at
|
||||
// death, the identified-armor name, and the detect-magic test.
|
||||
func TestMalformedArmorDoesNotPanic(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
armor := malformed(KindArmor)
|
||||
armor.Flags.Set(Known)
|
||||
|
||||
mustNotPanic(t, "naming a malformed suit of armor", func() {
|
||||
if got := g.inventoryName(armor, false); got != armor.Kind.String() {
|
||||
t.Errorf("inventoryName = %q, want %q", got, armor.Kind.String())
|
||||
}
|
||||
})
|
||||
|
||||
mustNotPanic(t, "isMagic on a malformed suit of armor", func() {
|
||||
g.isMagic(armor)
|
||||
})
|
||||
|
||||
mustNotPanic(t, "appraising a malformed suit of armor", func() {
|
||||
if worth := g.objectWorth(armor); worth != 0 {
|
||||
t.Errorf("objectWorth = %d, want 0", worth)
|
||||
}
|
||||
})
|
||||
|
||||
if got := g.data.armorClass(armor.Which); got != 0 {
|
||||
t.Errorf("armorClass(%d) = %d, want 0", armor.Which, got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestMalformedWeaponDoesNotPanic covers the init_dam[] read. WeaponFlame
|
||||
// is the first kind with no table row, so initWeapon must leave the
|
||||
// object alone rather than index past the end.
|
||||
func TestMalformedWeaponDoesNotPanic(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
weap := newObject()
|
||||
|
||||
mustNotPanic(t, "initWeapon with the flame pseudo-weapon", func() {
|
||||
g.initWeapon(weap, WeaponFlame)
|
||||
})
|
||||
|
||||
mustNotPanic(t, "initWeapon with a negative weapon kind", func() {
|
||||
g.initWeapon(weap, WeaponKind(-1))
|
||||
})
|
||||
|
||||
if weap.Kind != KindNone {
|
||||
t.Errorf("weapon was initialized from a missing table row: kind %v",
|
||||
weap.Kind)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFixStickMalformedWhichDoesNotPanic covers the ws_type[] read that
|
||||
// the reported reproducer actually crashed on.
|
||||
func TestFixStickMalformedWhichDoesNotPanic(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGameInput(t)
|
||||
wand := malformed(KindWand)
|
||||
|
||||
mustNotPanic(t, "fixStick on a malformed wand", func() {
|
||||
g.fixStick(wand)
|
||||
})
|
||||
|
||||
if wand.Damage.String() != "1x1" {
|
||||
t.Errorf("damage = %q, want the wand damage %q",
|
||||
wand.Damage.String(), "1x1")
|
||||
}
|
||||
}
|
||||
|
||||
// TestRestoreRejectsOutOfRangeWhich is the save-file half of the fix: a
|
||||
// malformed object must not be able to sneak past the keyboard guard by
|
||||
// arriving in a snapshot.
|
||||
//
|
||||
// A decoded save is also the only place a *negative* Which can come
|
||||
// from. On the keyboard path createObj's ch-'a' is byte arithmetic and
|
||||
// wraps, so 'A' and '!' land at 234 and 202; Which is a plain int in the
|
||||
// gob stream, so a tampered file can carry any value at all. Both shapes
|
||||
// are covered here, and the negative case is what exercises the
|
||||
// Which >= 0 arm of hasValidWhich.
|
||||
func TestRestoreRejectsOutOfRangeWhich(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
which int
|
||||
}{
|
||||
{"one past the wand table", int(NumWandTypes)},
|
||||
{"the value 'A' wraps to on the keyboard path", 234},
|
||||
{"the value '!' wraps to on the keyboard path", 202},
|
||||
{"negative, reachable only from a tampered file", -1},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
g := mkGame(t, 11)
|
||||
st := g.snapshot()
|
||||
|
||||
if len(st.Player.Body.Pack) == 0 {
|
||||
t.Fatal("starting pack is empty; nothing to corrupt")
|
||||
}
|
||||
|
||||
st.Player.Body.Pack[0].Kind = KindWand
|
||||
st.Player.Body.Pack[0].Which = tc.which
|
||||
|
||||
path := filepath.Join(t.TempDir(), "rogue.save")
|
||||
writeSnapshot(t, path, st)
|
||||
|
||||
_, restoreErr := Restore(path, Params{Term: &testTerm{}})
|
||||
if !errors.Is(restoreErr, ErrSaveCorrupt) {
|
||||
t.Errorf("Restore error = %v, want ErrSaveCorrupt", restoreErr)
|
||||
}
|
||||
|
||||
_, statErr := os.Stat(path)
|
||||
if statErr != nil {
|
||||
t.Error("a rejected save file was deleted; it should be left alone")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// writeSnapshot gob-encodes a snapshot to path the way saveFile does.
|
||||
func writeSnapshot(t *testing.T, path string, st *SaveState) {
|
||||
t.Helper()
|
||||
|
||||
f, err := os.Create(path) //nolint:gosec // G304: test temp path
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
encErr := gob.NewEncoder(f).Encode(st)
|
||||
if encErr != nil {
|
||||
t.Fatal(encErr)
|
||||
}
|
||||
|
||||
closeErr := f.Close()
|
||||
if closeErr != nil {
|
||||
t.Fatal(closeErr)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWhichLimitCoversEveryIndexedTable pins the bounds table itself
|
||||
// against the per-kind arrays it has to agree with.
|
||||
func TestWhichLimitCoversEveryIndexedTable(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// len() of an array field is a compile-time constant, so the zero
|
||||
// value is enough to read the table sizes off.
|
||||
var it ItemLore
|
||||
|
||||
cases := []struct {
|
||||
kind ObjectKind
|
||||
size int
|
||||
}{
|
||||
{KindPotion, len(it.Potions)},
|
||||
{KindScroll, len(it.Scrolls)},
|
||||
{KindRing, len(it.Rings)},
|
||||
{KindWand, len(it.Sticks)},
|
||||
{KindArmor, len(it.Armors)},
|
||||
{KindWeapon, len(it.Weapons)},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
if got := whichLimit(tc.kind); got != tc.size {
|
||||
t.Errorf("whichLimit(%v) = %d, want the table size %d",
|
||||
tc.kind, got, tc.size)
|
||||
}
|
||||
}
|
||||
|
||||
// Kinds whose Which is not a table index accept anything, as in C.
|
||||
for _, kind := range []ObjectKind{KindFood, KindAmulet, KindGold, KindNone} {
|
||||
obj := &Object{Kind: kind, Which: 99}
|
||||
if !obj.hasValidWhich() {
|
||||
t.Errorf("%v should not be bounds-checked on Which", kind)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user