nanny reads argument[0] of an empty reply at five lobby prompts (debug builds abort)

Written by whoever made the change (here, Claude Code working from the dossier), answering the dossier's questions. The facts it cites are in dossier.md; the proof is in gate.md.

In one paragraph

Five of the lobby's prompts (the questions a connecting player answers before playing) look at the first letter of the reply with argument[0], without checking there is one. argument has been a std::string_view since December 2021, and indexing an empty view is undefined behaviour, so pressing Enter at "Did I hear that right (Y/N)?" (or four other prompts) reads past the end. Since the project moved to GCC 15 on 2026-10-31, debug builds abort on the standard library's bounds assertion there, taking the whole MUD down for everyone connected. Release builds get away with it, by luck rather than design: the byte past the end happens to be a NUL terminator. The fix reads the first letter through a tiny first_char() that returns '\0' for an empty reply, so each prompt falls into the default: branch it already has and simply asks again, as the ANSI prompt already does for an empty reply.

Root cause

Why debug builds abort and release builds don't

The same reason as #1, checked the same way with g++ 15.2: at -O0 libstdc++ checks operator[] and aborts; at -O2 (and at -O0 with -D_GLIBCXX_NO_ASSERTIONS) the read goes ahead silently, and AddressSanitizer doesn't report it, because the byte after the view is still inside the input buffer. So it is undefined behaviour in every build, and the assertion is only what makes it visible.

What release builds actually read: every caller passes a view of something NUL-terminated (the std::string pop_incomm returns, or the literal "" on reconnect), and ltrim only moves the start of the view. So the byte just past an empty view is always a '\0', which matches no case and lands in default:, the right answer by accident. (The issue said release builds "switch on whatever byte follows"; in practice that byte is always the terminator. The optimiser is still entitled to assume the read never happens, which is why it's worth fixing anyway.)

One contrast that explains why only these lines are affected: Descriptor::pop_incomm does next_line[0] on the same empty line, but next_line is a std::string, and s[s.size()] on a std::string is defined (it's the terminator). Only std::string_view has no terminator to read.

A second way in, found while checking callers

Not every empty reply comes from a player. On a doorman reconnect (PACKET_RECONNECT, when doorman hands xania back an already-authenticated connection, e.g. after xania restarts), handle_doorman_packet calls nanny(d, "") itself, twice (MudImpl.cpp:506 and 509). If the first call finds the character already playing on another connection, check_playing moves the descriptor to BreakConnect, and the second call indexes the empty literal there: a debug MUD would abort with no player input at all. This is from reading the code; it hasn't been reproduced (Sim would need to restart xania under a live doorman). The fix covers it the same way: BreakConnect gets '\0' and answers "Please type Y or N?".

The fix

// The first character of str, or '\0' if str is empty. For code that switches on the first letter of a reply,
// where str[0] on an empty string_view would read past its end.
[[nodiscard]] constexpr char first_char(std::string_view str) noexcept { return str.empty() ? '\0' : str.front(); }

in string_utils.hpp, next to is_vowel and has_prefix, and in the five prompts:

switch (first_char(argument)) {                       // BreakConnect, ConfirmNewName, GetAlignment, DefaultChoice
if (auto sex = Sex::try_from_char(first_char(argument))) {   // GetNewSex

'\0' matches no case, so each prompt does what it already does for a reply it doesn't understand:

That's what the C version did before 03e4dc4, so the behaviour is restored, not invented.

Alternatives considered

• An if (argument.empty()) block in each of the five states, as GetAnsi has: five copies of a re-prompt that each default: already writes, and five more places for them to drift apart. Rejected.
• switch (argument.empty() ? '\0' : argument[0]) inline: the same idea without a name, repeated five times. A named helper says why and can be tested.
• Reject empty lines before nanny (in pop_incomm or the game loop): would also change states where an empty line means something (GetAnsi treats it as "no"; the MOTD's "press return"), and wouldn't cover the reconnect path, which calls nanny directly. Rejected.
• Refactor nanny (444 lines; upstream #279 proposes a Lobby class): the right long-term home for this, but not in a bug fix. first_char will move with the code.

Answers to the dossier's questions

1. Explorer. No Explorer check covers this yet; code.lobby-empty-reply is proposed (separate tool, separate change), which would list any of the five lines still indexing argument[0].

2. Architecture. Local to MudImpl::nanny, plus a header-only helper. Every caller of nanny benefits: the game loop (player input) and both doorman reconnect calls.

3. Design patterns. string_utils.hpp is where small string_view helpers live (is_vowel, has_prefix, matches); first_char is constexpr and noexcept like the trivial ones. Each prompt's existing default: is reused instead of a new branch.

4. Technical debt. Pays down the five unguarded reads. Leaves GetAnsi's argument[0] (line 762) as it is: it's guarded by the empty() check above it, and changing it would only be churn. (Builder's detector still lists it; it doesn't yet understand guards.) The same pattern elsewhere: do_get is #1; do_who and do_permit are guarded (false positives); skill_lookup (practice '') is an unverified lead, noted in #1's analysis.

5. Gameplay. A player who presses Enter at one of the five prompts is asked again instead of crashing a debug MUD. Release builds already asked again (by accident, see above), so on the live game nothing visible changes. Nothing new for an abusive player: before the fix, pressing Enter at a prompt could take a debug MUD down for everyone, which was the real exploit. No balance numbers involved.

6. Documentation. None needed: the prompts' messages already exist and are what players see.

7. Tests. first_char gets a unit test (including an empty view into a longer string, the case that bites, and a static_assert). nanny is private to MudImpl and needs a whole MUD, so the end-to-end proof is Xania Sim's lobby trial: a careless player pressing Enter at each of the five prompts. The gate runs it on the base (it should crash at every prompt) and on the branch (every prompt must be reached and survive), and the before/after proof comes from that.

8. History. Answered under Root cause. Upstream: no issue about empty replies; upstream #279 (open) wants nanny broken up; upstream #129 (closed, "Splat in reconnect") and upstream #178 (open) are about the reconnect path, which this also hardens.

Follow-ups (not in this PR)

• Xania Explorer: the code.lobby-empty-reply check.
• Xania Sim: a reconnect trial (restart xania under a live doorman with a character logged in twice) to confirm the second path.
• Builder: teach the detector about empty() guards (it flags line 762 and two other guarded places).

Branch fix/3-lobby-empty-reply at f7a0c3b52d against base 82171fa443: passed in 3 min, 2026-11-11 11:22.

Environment: gcc g++-15 (Ubuntu 15.2.0-16ubuntu1) 15.2.0; cmake cmake version 4.2.3; os Ubuntu 26.04.1 LTS; distro Ubuntu-26.04

Changes: 3 files changed, 23 insertions(+), 5 deletions(-)

• f7a0c3b nanny: an empty reply at a Y/N-style lobby prompt asks again instead of reading past the end (#3)

The new tests, before and after

Detectors, base against branch

Logs

smoke: Smoke: create a bot, look, quit (20 s, Xania f7a0c3b52d)
  transcript: runs\20261111-112305\Smokea.log
  result: clean

[00:37] lobby GetNewSex: xania died (prompt=Are you male, female or other (M/F/O)? reply=)
[00:50] lobby GetAlignment: walking to the prompt
[00:56] lobby GetAlignment: xania died (prompt=Which alignment (G/N/E)? reply=)
[01:09] lobby DefaultChoice: walking to the prompt
[01:15] lobby DefaultChoice: xania died (prompt=Customize (Y/N)? reply=)
[01:28] lobby BreakConnect: walking to the prompt
[01:37] lobby BreakConnect: xania died (prompt=Do you wish to connect anyway (Y/N)? reply=)
lobby: Lobby: an empty reply at each login prompt (112 s, Xania 82171fa443)
  ConfirmNewName crashed, GetNewSex crashed, GetAlignment crashed, DefaultChoice crashed, BreakConnect crashed
  failed: lobby: ConfirmNewName: XANIA DIED after an empty reply to "Did I hear that right - 'Lobbya' (Y/N)?"
  failed: lobby: GetNewSex: XANIA DIED after an empty reply to 'Are you male, female or other (M/F/O)?'
  failed: lobby: GetAlignment: XANIA DIED after an empty reply to 'Which alignment (G/N/E)?'
  failed: lobby: DefaultChoice: XANIA DIED after an empty reply to 'Customize (Y/N)?'
  failed: lobby: BreakConnect: XANIA DIED after an empty reply to 'Do you wish to connect anyway (Y/N)?'
  [known] high xania crashed (SIGABRT, failed assertion) in MudImpl::nanny (MudImpl.cpp)
  [known] high xania crashed (SIGABRT, failed assertion) in MudImpl::nanny (MudImpl.cpp)
  [known] high xania crashed (SIGABRT, failed assertion) in MudImpl::nanny (MudImpl.cpp)
  [known] high xania crashed (SIGABRT, failed assertion) in MudImpl::nanny (MudImpl.cpp)
  [known] high xania crashed (SIGABRT, failed assertion) in MudImpl::nanny (MudImpl.cpp)
  crash_ConfirmNewName: runs\lobby-20261111-112312\xania-crash-ConfirmNewName.txt
  crash_GetNewSex: runs\lobby-20261111-112312\xania-crash-GetNewSex.txt
  crash_GetAlignment: runs\lobby-20261111-112312\xania-crash-GetAlignment.txt
  crash_DefaultChoice: runs\lobby-20261111-112312\xania-crash-DefaultChoice.txt
  crash_BreakConnect: runs\lobby-20261111-112312\xania-crash-BreakConnect.txt
  result: clean

[00:14] lobby ConfirmNewName: walking to the prompt
[00:19] lobby ConfirmNewName: survived (prompt=Did I hear that right - 'Lobbya' (Y/N)? reply=It's quite simple - type Yes or No:)
[00:19] lobby GetNewSex: walking to the prompt
[00:25] lobby GetNewSex: survived (prompt=Are you male, female or other (M/F/O)? reply=Please specify (M)ale, (F)emale or (O)ther.)
[00:25] lobby GetAlignment: walking to the prompt
[00:31] lobby GetAlignment: survived (prompt=Which alignment (G/N/E)? reply=That's not a valid alignment.
Which alignment (G/N/E)?)
[00:31] lobby DefaultChoice: walking to the prompt
[00:38] lobby DefaultChoice: survived (prompt=Customize (Y/N)? reply=Please answer (Y/N)?)
[00:38] lobby BreakConnect: walking to the prompt
[00:47] lobby BreakConnect: survived (prompt=Do you wish to connect anyway (Y/N)? reply=Please type Y or N?)
lobby: Lobby: an empty reply at each login prompt (50 s, Xania f7a0c3b52d)
  ConfirmNewName still up, GetNewSex still up, GetAlignment still up, DefaultChoice still up, BreakConnect still up
  result: clean

#3 · Xania 82171fa443 · 2026-11-11 10:11

/usr/include/c++/15/string_view:260: ... operator[](size_type) const ...: Assertion '__pos < this->_M_len' failed.
Program received signal SIGABRT, Aborted.

case DescriptorState::ConfirmNewName:
    if (argument.empty()) {
        d->write(fmt::format("Did I hear that right - '{}' (Y/N)? ", ch->name));
        break;
    }
    switch (argument[0]) {

Focus

• MudImpl::nanny (src/MudImpl.cpp:603, 444 lines): named in the idea
• Descriptor::pop_incomm (src/Descriptor.cpp:60, 21 lines): named in the idea

1. Explorer insights

• Bug check code.lobby-empty-reply (present, crash): An empty reply at a login prompt crashes debug builds. MudImpl.cpp:711, MudImpl.cpp:740, MudImpl.cpp:854, MudImpl.cpp:890, MudImpl.cpp:915

To decide

• The fix should turn the code check above to fixed (it matches the faulty pattern). Does it, and are there other occurrences to fix at the same time?

2. Architecture

• MudImpl::nanny is defined at src/MudImpl.cpp:603-1046 (444 lines): void MudImpl::nanny(Descriptor *d, std::string_view argument)
• Called from MudImpl::game_loop (src/MudImpl.cpp:180), MudImpl::handle_doorman_packet (src/MudImpl.cpp:483)
• nanny calls ArgParser (3 definitions), act (7 definitions), all (2 definitions), announce, bug (2 definitions), by_name, channel, char_to_room, character (3 definitions), check_ban (2 definitions), check_enum_bit, check_playing and 60 more
• nanny reaches persisted or shared state: inventory
• Descriptor::pop_incomm is defined at src/Descriptor.cpp:60-80 (21 lines): std::optional<std::string> Descriptor::pop_incomm()
• Called from MudImpl::game_loop (src/MudImpl.cpp:180)
• pop_incomm calls pop_raw, sanitise_input, size (2 definitions), write_direct
• src/MudImpl.cpp: 1114 lines, 44 functions
• src/Descriptor.cpp: 322 lines, 26 functions

To decide

• Is the change local to these functions, or does the fix belong in a shared helper their callers use too?
• Does every caller (including automatic ones) still get the behaviour it expects?

3. Design patterns

• In nanny: ArgParser x3 (modern), std::string_view x1 (modern), fmt::format x5 (modern), matches()/matches_start() x4 (modern), range-for over contents x1 (modern), act() x2 (either), '\0' terminator checks x1 (legacy)
• nanny mixes modern (ArgParser, std::string_view, fmt::format, matches()/matches_start(), range-for over contents) and legacy ('\0' terminator checks) idioms: the modern ones already in the function are the pattern to follow.
• Across src/: act() 779, std::string_view 625, ArgParser 402, matches()/matches_start() 291, range-for over contents 267, fmt::format 211, sprintf/strcpy/strcat 19, '\0' terminator checks 14, char buffers 10

To decide

• Which existing idiom does the change follow, and does it leave the function more consistent?
• Is there a helper already (in the same file or a header) that does what the change needs?

4. Technical debt

• nanny indexes string_views like C strings (reading past the end when the view is that long, and never seeing a terminator): argument[0] at src/MudImpl.cpp:711, argument[0] at src/MudImpl.cpp:740, argument[0] at src/MudImpl.cpp:762, argument[0] at src/MudImpl.cpp:854, argument[0] at src/MudImpl.cpp:890, argument[0] at src/MudImpl.cpp:915
• nanny is 444 lines long (src/ median 7).
• Legacy idioms in nanny: '\0' terminator checks
• TODO at src/MudImpl.cpp:28: remove this
• TODO at src/MudImpl.cpp:44: move these?
• TODO at src/MudImpl.cpp:57: Move into a Universe class
• TODO at src/MudImpl.cpp:96: do something rather than return here if there's a failure
• TODO at src/MudImpl.cpp:456: remove global
• TODO at src/MudImpl.cpp:668: crypt can return null if if fails (e.g. password is truncated).
• The same pattern elsewhere in src/ (10 places in 4 functions; each is the same kind of bug unless the view is known to be long enough): do_who arg[0] :1249; do_get arg1[3] :641, &arg1[4] :641, arg1[3] :648, &arg1[4] :651, arg1[3] :707; do_permit player[0] :95, player[0] :98; skill_lookup name[0] :39, name[0] :39

To decide

• Are the other places with the same pattern the same bug, and do they belong in this change?
• What debt does the change pay down while it's here, and what does it deliberately leave?
• Does the change add any (a special case, a duplicated check, a new legacy idiom)?

5. Gameplay

• Messages nanny can show: "Nanny: bad d->state() {}."; "Illegal name, try another.\n\rName: "; "Denying access to {}@{}."; "You are denied access.\n\r"; "The game is wizlocked. Try again later - a reboot may be imminent.\n\r"; "Password: "; "The game is newlocked. Try again later - a reboot may be imminent.\n\r"; "Your site has been banned.\n\r"; "Did I hear that right - '{}' (Y/N)? "; "Our survey said <Crude buzzer noise>.\n\rWrong password.\n\r" (+49)
• Messages pop_incomm can show: "Line too long.\n\r"
• Balance numbers Explorer models from these files: none (the change is behaviour, not balance; Explorer's predictions shouldn't move)

To decide

• What does a player notice before and after (messages, what they get, what's refused)?
• Can a careless or abusive player use the change (or the old behaviour) to their advantage?
• Does it change balance for any class, race or level range? If so, which Explorer numbers?

6. Documentation

• No help entries or docs mention the focus.

To decide

• Does any help entry describe the behaviour being changed, and does it need rewording?
• Does the change need a note anywhere else (README, release notes, an issue comment)?

7. Test coverage

• src/apps/test/pfutest.cpp: includes MudImpl.hpp
• src/test/save_test.cpp: includes MudImpl.hpp
• src/test/DescriptorTest.cpp: named after Descriptor.cpp, includes Descriptor.hpp
• No unit test calls nanny.
• No unit test calls pop_incomm.
• Sim finding crash:SIGABRT:MudImpl.cpp:MudImpl::nanny [known, seen 5x]: xania crashed (SIGABRT, failed assertion) in MudImpl::nanny (MudImpl.cpp) (https://github.com/abraidotti/xania/issues/3)

To decide

• Which unit test pins the fixed behaviour (a test that fails before the change)?
• Which Sim trial or scenario shows it from a player's side, and should a new one be added?

8. History

• argument is declared at src/MudImpl.cpp:603, a line last changed in 687f6ce (2025-05-11, snellers): Step 2 of the next big refactoring project.
• argument[0] at src/MudImpl.cpp:711 was last changed in 03e4dc4 (2021-12-05, snellers): nanny() lobby func upgraded to use string_view....(#263)
• argument[0] at src/MudImpl.cpp:740 was last changed in 03e4dc4 (2021-12-05, snellers): nanny() lobby func upgraded to use string_view....(#263)
• argument[0] at src/MudImpl.cpp:762 was last changed in 03e4dc4 (2021-12-05, snellers): nanny() lobby func upgraded to use string_view....(#263)
• argument[0] at src/MudImpl.cpp:854 was last changed in 0fbb3f4 (2021-03-31, snellers): * New Sex wrapper class and enum:
• argument[0] at src/MudImpl.cpp:890 was last changed in 7a71102 (2020-07-22, Matt Godbolt): Reformat all the code with clang-format
• argument[0] at src/MudImpl.cpp:915 was last changed in 7a71102 (2020-07-22, Matt Godbolt): Reformat all the code with clang-format
• The conversion to string_view (or ArgParser): 03e4dc4 (2021-12-05, snellers): nanny() lobby func upgraded to use string_view....(#263). The indexing above has been on a view, with no terminator, since then
• nanny (444 lines) comes from 55 commits; the most: 33a9553 (2020-07-05, Matt Godbolt): Initial import of just code, areas, and html (178 lines); 03e4dc4 (2021-12-05, snellers): nanny() lobby func upgraded to use string_view....(#263) (44 lines); e4a77fe (2020-08-08, Matt Godbolt): Replace free function with call to d->write() throughout (28 lines); f2c8558 (2022-04-03, snellers): Char creation lobby is colourized to improve usability (detect ansi early). (20 lines)
• pop_incomm (21 lines) comes from 2 commits; the most: d5e56a6 (2020-08-07, Matt Godbolt): Descriptor refactor (#106) (20 lines); f2ee4e1 (2020-08-08, Matt Godbolt): Remove outbuf and outsize (1 lines)
• Since 03e4dc4 (2021-12-05), 26 commits changed the compiler or its flags; the latest: 7bd8c82 (2026-10-31, snellers): github action upgrade to Ubuntu 26.04 and g++-15; 82823ff (2026-10-31, snellers): General tooling upgrades, largely for Ubuntu 26.04.1 compatibility conda 26.9.4-0 conan 2.33.0 cmake >= 3.31 g++-15 default, g++-13 fallback Compiles on ubunt 26.04.1; 425fd97 (2025-04-07, snellers): Docker: ubuntu 24 is the default these days. Install clang 19. Stop trying to install awscli as it's not in the ubuntu apt sources now. The session doesn't start tmux by default. [skip ci]; e5483b6 (2025-03-28, snellers): Update g++ versions in README. Retire the g++ 10/11 toolchains.; 55c9b1b (2025-03-28, snellers): github: upgrade to ubuntu 24 and use g++-13
• Debug builds add -fno-omit-frame-pointer -fsanitize=leak -fsanitize=address -fsanitize=undefined
• Release builds add -O2 -DNDEBUG -Wno-error=array-bounds
• Upstream issue #280 (closed): Duplicate characters during char creation (mentions nanny)
• Upstream issue #279 (open): Breakup & refactor nanny() (mentions nanny)
• Upstream issue #129 (closed): Splat in reconnect (mentions nanny)
• Upstream issue #127 (closed): Heap use after free in xania (mentions nanny)
• Upstream issue #38 (closed): MUD should time out inactive connections pre-Play (mentions nanny)

To decide

• When did the behaviour start, and why did nobody notice until now?
• Did a toolchain change (compiler, flags, sanitizers) make it show, or change how it shows?
• Has upstream seen or tried this before, and does the change fit what was decided then?

Gate plan

What prototype runs on the fork before this can ship:

• Builds: Debug (ASan, UBSan, LSan, libstdc++ assertions), Release (-O2)
• Unit tests: src/apps/test/pfutest.cpp, src/test/DescriptorTest.cpp, src/test/save_test.cpp
• Sim: smoke: the game still boots and a player can log in
• Sim: churn --minutes 5: logins and quits reach it (lifetime bugs show here)
• Regression: Sim finding crash:SIGABRT:MudImpl.cpp:MudImpl::nanny should stop reproducing (https://github.com/abraidotti/xania/issues/3)