From 35ddd0ad2215760bb1d3dc7724480f179ea33fa6 Mon Sep 17 00:00:00 2001 From: historia <[not public]> Date: Wed, 16 Sep 2026 21:05:39 -0400 Subject: fix: login account race, casino chip double-refund, room-create rollback, rename collision, fatal transport errors --- internal/game/core_login_account.go | 51 ++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 20 deletions(-) (limited to 'internal/game/core_login_account.go') diff --git a/internal/game/core_login_account.go b/internal/game/core_login_account.go index 371c287..f6e5631 100644 --- a/internal/game/core_login_account.go +++ b/internal/game/core_login_account.go @@ -19,6 +19,7 @@ func (g *Game) handleAccountName(sess *net.Session, input string) { actualName, err := g.AccountStore.FindAccount(name) if err == nil { sess.Account = &player.Account{Name: actualName} + sess.SetAuthName(actualName) sess.State = net.StatePassword sess.SetEcho(false) sess.Write("Password: ") @@ -29,6 +30,7 @@ func (g *Game) handleAccountName(sess *net.Session, input string) { return } sess.Account = &player.Account{Name: name} + sess.SetAuthName(name) sess.State = net.StateNewAccountPass sess.SetEcho(false) sess.WriteLine("\r\nOh, a new visitor to this rock!") @@ -99,17 +101,9 @@ type passwordAuth struct { // The production entry point; handlePassword stays for direct HandleSession // callers (tests) but hashes on the owner there. func (g *Game) handlePasswordOffOwner(sess *net.Session, input string) { - if sess.Account == nil { - // Defensive: password prompts only follow an account-name step. - g.RunOnOwner(func() { - if g.isShuttingDown() { - return - } - g.handlePassword(sess, input) - }) - return - } if input == "" || len(input) > 128 { + // These checks touch no session state, so they are race-free off + // the owner; they also keep Argon2id off absurd inputs. tooshort := input == "" g.RunOnOwner(func() { if g.isShuttingDown() { @@ -127,8 +121,24 @@ func (g *Game) handlePasswordOffOwner(sess *net.Session, input string) { }) return } + // The account name comes from the race-free authName snapshot (set by + // the owner when the password prompt was offered), NOT from + // sess.Account: the owner goroutine can replace Account at any moment + // (account republish after a rename/purge), and reading a plain field + // the owner concurrently writes is a data race. + name := sess.AuthName() + if name == "" { + // Defensive: password prompts only follow an account-name step. + g.RunOnOwner(func() { + if g.isShuttingDown() { + return + } + g.handlePassword(sess, input) + }) + return + } auth := passwordAuth{} - acc, err := g.AccountStore.LoadAccount(sess.Account.Name) + acc, err := g.AccountStore.LoadAccount(name) switch { case err != nil: auth.loadErr = true @@ -251,20 +261,21 @@ func (g *Game) handleNewAccount(sess *net.Session, input string) { // for the existence re-check and save. func (g *Game) handleNewAccountOffOwner(sess *net.Session, input string) { input = strings.TrimSpace(input) - if input == "" || input != sess.PendingPass { - g.RunOnOwner(func() { - if g.isShuttingDown() { - return - } - g.handleNewAccount(sess, input) - }) - return - } + // Hash unconditionally on the connection goroutine (Argon2id is + // expensive); the owner task below re-checks the empty/mismatch cases + // against owner-owned session state, so PendingPass is never read off + // the owner. A wasted hash only happens on the rare mismatched-confirm + // path, where handleNewAccount's own validation rejects it without a + // second hash. hash, hashErr := player.HashPassword(input) g.RunOnOwner(func() { if g.isShuttingDown() { return } + if input == "" || input != sess.PendingPass { + g.handleNewAccount(sess, input) + return + } g.applyNewAccount(sess, hash, hashErr) }) } -- cgit v1.2.3