Five call sites computed English plural morphology and passed it as a %s
argument:
tprintf(M_("%d connection%s closed."), count, (count == 1 ? "" : "s"))
That shape can only ever express English. A translator gets a %s slot
with nothing to put in it: dropping the argument makes msgfmt -c reject
the entry and desynchronises the varargs cursor; keeping it emits a stray
Latin "s" whenever count != 1. Korean has no plural inflection at all,
Russian and Polish have three forms, Arabic six.
MN_(singular, plural, n) sends the COUNT to the catalogue instead, and
the catalogue's own Plural-Forms rule chooses. Three distinct msgids
across five sites.
## The MSVC catalogue path deliberately does not translate plurals
It returns English. Choosing a form means parsing Plural-Forms out of
the .mo header and evaluating the expression, and that reader does
neither. Guessing form 0 would be right for the nplurals=1 languages and
quietly wrong for every other -- Russian would get the singular for 2..4.
English is wrong in a way a translator can see and report; a wrong
Russian form is not. The real gettext path, which every platform with
libintl takes, does it properly.
## Two guards needed teaching, and both had real bugs
check_nls.py treated msgid_plural as a CONTINUATION of msgid, welding the
two originals into one key that exists nowhere:
'stale msgid "**** %d failed connect ...****[xx] **** %d failed
connect%s ...****[xx] ****..."'
Every downstream check was then comparing against a msgid that does not
exist. It now parses msgstr[N] as a list of forms, requires every form
to carry the msgid's conversion sequence, and reports a partially
translated plural entry -- some forms filled, some empty -- which
previously counted as translated and would render blank at runtime.
check_formats.py read MN_() as a non-constant format and failed the
build. It could not simply join CONST_CAST_WRAPPERS: the third argument
is a runtime count, so the expression genuinely is not literal-only. It
now rewrites MN_(s,p,n) to (s p) -- adjacent literals, which is C++
concatenation and which _literal_only already accepts. Joined with a
space rather than a comma, because a comma is not in that check's allowed
character set and would have failed the very thing being enabled. Both
forms therefore reach the conversion scan; each is a format string in its
own right.
Verified non-vacuous: a bad conversion in the PLURAL form is caught, and
a genuinely non-literal format inside MN_() is still rejected.
## The pseudo-locale needed rebuilding, not merging
msgmerge fuzzy-matched the OLD msgids into the new plural entries, so
xx.po carried translations still containing the %s the new originals do
not have. Left alone they would have failed the conversion check the
moment the fuzzy marker came off. Rebuilt from each entry's own msgids,
and xx.po gained an explicit Plural-Forms so the next merge does not
guess.
ko.po needed no invented Korean: those three were already msgstr "",
exactly consistent with this issue's claim that they were untranslatable
in the old shape.
## Runtime proof
tests/nls/run.sh gains two cases. The existing three could not have
shown this -- both forms are translated, so a build that always returned
msgstr[0], or one where MN_() fell through to the English ternary, would
score identically:
plural: n=1 picks singular ok [xx] 1 entrance found.
plural: n>1 picks plural ok [xx] 2 entrances found.
The count is controlled rather than observed: @entrances against a
freshly dug room gives exactly 1, and one more exit gives exactly 2.
Reading it against the starter database would have made the case depend
on that database's exit topology.
make test green: 1561/1561 on all three smoke routes, format guard 1170
call sites clean, NLS guard 653 msgids clean, NLS runtime 5/5.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Continue Phase 3 after create/set/wiz: mark player/staff tprintf and
safe_tprintf format templates with M_() so whole sentences (look examine
fields, say/page/whisper, login/alias/protect notices) enter the catalogue.
Left as T(): softcode decompile templates (@create/@dig/@lock…), ANSI color
attribute assemblies, pure name+message glue ("%s %s"/"%s%s"), machine
storage ("#%d", logindata, XX hash), and log_printf diagnostics.
Regenerate pot/xx/mo without empty msgstr. Format and NLS guards green.
Next notify slice: page/whisper/gag denials in speech.cpp and look/examine
chrome in look.cpp (~36 sites). Leave tprintf formats and HTML fragments
as T(). Half-mark pass promotes Contents: twin. Regenerate pot/xx.
Replace ~900 typographic \xE2\x80\x.. escapes (curly quotes, en dashes)
and a few \xE2\x80\230 octal workarounds with real UTF-8 in message
strings under mux/modules and mux/src. Leave stringutil and convert
charset mapping tables as explicit byte sequences.
Completes the sweep the issue called for. mux_atol returns long, which
is 32-bit on LLP64, so every caller silently truncated on Windows. Two
of those were real defects (the truthiness family and cf_size, fixed in
the preceding commits); the rest were latent, waiting for a value large
enough to matter.
Rather than audit 290 sites for whether each can reach 2^31 today, use
the 64-bit parser everywhere and remove the class. A dbref cannot
overflow now, but nothing stops a later caller passing that same site a
timestamp or a byte count.
Pure 1:1 substitution: 285 lines changed, and every removed line
contained mux_atol while every added line contains mux_atoi64. No
control flow, no types, no behaviour beyond the wider parse.
This is a NO-OP on LP64 -- long is already 64-bit on Linux and macOS, so
the generated code there is unchanged. It only widens the parse on
Windows. Narrowing destinations are unaffected either way: `int x =
mux_atoi64(s)` truncates exactly as `int x = mux_atol(s)` did, on both
models.
Left alone: mux_atol itself in mathutil, its declaration, and three
comments that name it. Callers that genuinely want 32-bit semantics can
still ask for them; none appear to.
Verified on Windows: full solution builds clean with no new warnings,
smoke is 1418 passed / 16 failed / 0 crashes / 306 of 306 dispatched --
identical to before the sweep, with the same 16 build-configuration
failures (exp3 module not loaded, hmac/digest behind UNIX_DIGEST).
Spot checks after the change: the boolean family returns 1 for multiples
of 2^32, cf_size round-trips 3000000000 and still reads -1 as unlimited,
and arithmetic, string and list functions are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four unclaimed Pass 8 findings in four separate files, chosen to avoid the
files the open PR queue is already touching. Each premise was re-verified
against current source before fixing.
#1182 p6h_vaht_crypt OOB read (player.cpp). The guard only required
szSetting to be at least the 13-byte "$P6H$$1:sha1:" prefix, but the
timestamp is copied from a constant offset 54 bytes in (prefix + 40 hex
digits + separator). Any A_PASS carrying that prefix with a total length
of 13..53 passed the check and then ran safe_str off the end of the
attribute value. Reachable because mux_crypt classifies anything starting
"$P6H$" (and not "XX") as CRYPT_P6H_VAHT, so a truncated or corrupt A_PASS
-- raw attribute write or a damaged import, not @password -- reaches it on
the login path. Require the whole fixed layout including the separator,
and fail closed to szFail.
#1184 CONNECTED leak via decode_flags (flags.cpp). has_flag() and
flag_description() both hide the 'c' letter on Hidden(target) &&
!See_Hidden(player). decode_flags() required (WIZARD | DARK) together, so
every dark non-wizard -- royalty, staff, any mortal able to set itself DARK
-- still emitted 'c' to examiners. Hidden(x) is exactly (Flags(x) & DARK),
and decode_flags takes a FLAGSET rather than a dbref, so the aligned test
is DARK on the caller's flagset (unparse_object passes the target's).
#1186 moniker injection in look_contents (look.cpp). look_exits()
html_escape()s exit names inside xch_cmd="...", and the anchor text in
look_contents() was already escaped, but CONTENTS_LOCAL and CONTENTS_NESTED
inserted Moniker() raw into the attribute. A moniker containing a double
quote closed the attribute early and let the rest become further
Pueblo/HTML markup for HTML-capable clients. CONTENTS_REMOTE was already
safe (it emits #dbref).
#1187 page_check charged before validating (speech.cpp). payfor() ran
first; the not-connected and both A_LPAGE lock failures then returned false
with no refund, so a page that was never delivered still cost page_cost --
once per recipient, since do_page() calls page_check() per target.
Reordered to validate first and charge last; payfor() deducts only on
success, so no refund path is needed. The wizard "can't return your page"
warning is now held until after payment, so a sender who cannot afford the
page is not told about one that never happens.
Behaviour change worth noting: when a sender both lacks funds and the
target is offline, the message is now "Sorry, X is not connected." rather
than "You don't have enough coins." -- the actual reason rather than the
one that happened to be checked first.
Adds tests/scenario/page_cost.py (wired into run.sh) for #1187, the only
one of the four reachable without an HTML client or manufactured
connection state. It needs a mortal sender, since payfor() exempts
wizards outright and a Wizard-only test would pass against any
implementation.
Test validated against the unfixed build: cases 2 and 4 fail there
(offline page charged 5; two offline recipients charged 10, showing the
per-recipient amplification) while case 3 passes in both -- so it pins the
charge, not merely the absence of one.
The other three are not smoke-reachable: #1182 needs a crafted A_PASS,
#1184 needs live CONNECTED state on a dark player, #1186 needs an HTML
client. Their normal paths are covered -- every scenario driver logs in
through mux_crypt, and all four suites pass.
Verified: build clean, smoke 1404/1404 0 crashes, scenario 4/4 drivers
(wild_capture, site_threshold, jit_perms, telnet_negotiation) plus the new
page_cost 4/4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Memory-safety pass over previously unsurveyed engine subsystems. Six
confirmed, adversarially-verified bugs, all reachable from untrusted
player input:
- scramble()/shuffle(): Fisher-Yates index arrays were sized LBUF_SIZE/2
but the cluster/word count can reach LBUF_SIZE, so a ~32 KB argument
overflowed the stack array. Both are CA_PUBLIC. (#845)
- view_atr()/flags(): decode_attr_flags() can emit NUM_ATTRIBUTE_CODES+1
bytes but two callers gave it an 11-byte buffer. (#846)
- do_reference()/absolute_named_reference(): used snprintf's return value
(intended length) directly as a copy/compare length, reading past the
buffer on truncation; @reference could leak the adjacent heap bytes back
via @reference/list. Clamped to min(n, LBUF_SIZE-1). (#847)
- @cron value/step parsing accumulated digits into a signed int without an
overflow guard (UB). (#848)
- index(): trailing-space trim decremented before testing, reading the byte
before the lbuf when an item begins at the delimiter. (#849)
Adds regression tests TC012-TC014 to overflow_inject_fn.mux. Build clean,
all 1258 smoke tests pass. Coverage recorded in
docs/survey-memsafety-pass-2026-06.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add LBuf class to alloc.h: an RAII wrapper around alloc_lbuf/free_lbuf
that moves LBUF_SIZE buffers from the stack to the heap pool. Convert
all 108 non-static UTF8 xxx[LBUF_SIZE] stack arrays across 25 source
files. Static BSS buffers (24) are unchanged.
This eliminates LBUF_SIZE from recursive stack frames, making it safe
to increase LBUF_SIZE without risking stack overflow in the evaluation
pipeline.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
