Classic bug: Ship-selection output overflow

Bug ID	GE-BUG-022
Affected versions	Classic 3.2e and published classic source when a player owns more than 15 ships
Status	Verified through multiplayer reproduction and source analysis
Fixed in	ge-next 0.3
Primary source	GEFUNCS.C: lookupshp(), findships(), and selectship(); GEMAIN.H: SHIPTAB, SCANTAB, and NOSCANTAB

Summary

Classic reuses each player's 15-entry sensor scan table as temporary storage for the list of persistent ships shown when that player enters the game. findships() does not stop after 15 ships even though NOSCANTAB is 15. A player who owns a sixteenth ship therefore writes beyond that player's SCANTAB and into the scan table for the next channel.

The same list is printed without pagination, and selectship() accepts any nonnegative selection without checking it against NOSCANTAB. This is not merely a display-width problem: the list construction performs out-of-bounds writes into shared game memory, and the selection path later performs corresponding out-of-bounds reads.

Why the default configuration usually hides it

Classic permits MAXSHIPS, the maximum number of persistent ships one player may own, to be configured from 1 through 50. Its default is eight. The temporary selection table has a separate fixed limit:

#define NOSCANTAB 15

SCANTAB {
        SHIPTAB ship[NOSCANTAB];
};

An unmodified game using the default eight-ship limit cannot reach the overflow. The configuration file nevertheless advertises and accepts limits well above 15. Raising MAXSHIPS makes a supported configuration unsafe as soon as a player accumulates enough ships.

Why a sensor table is used for ship selection

SCANTAB normally holds the temporary sensor view belonging to one live ship slot. Its 15 SHIPTAB entries associate detected ships with scan letters, distances, bearings, headings, and speeds. Classic allocates these tables as one contiguous array:

scantab = (SCANTAB *)alcmem(n = nships * sizeof(SCANTAB));

During login, lookupshp() calls findships() to find every persistent ship owned by the user. Instead of using a dedicated ship selection array, findships() borrows the current channel's sensor table. For every matching database record it prints a row and stores the persistent ship number:

sptr = &scantab[usrnum];
found = 0;

do {
        if (sameas(usaptr->userid,warsptr->userid)) {
                /* print this ship as selection found + 1 */
                sptr->ship[found].shipno = warsptr->shipno;
                ++found;
        }
} while (qnxbtv());

There is no found < NOSCANTAB condition. The database walk ends only after every ship belonging to that user has been listed.

Where the writes go

C arrays are contiguous. Because SCANTAB contains only its 15-element ship array, the expression scantab[u].ship[15] addresses the same memory as scantab[u + 1].ship[0]. Only the shipno member of each out-of-range entry is written; the neighboring entry's old flag, letter, distance, bearing, heading, and speed remain in place.

Ships being listed	Destination of the stored ship numbers
1 through 15	The requesting player's selection table
16 through 30	All 15 entries belonging to the next channel
31 through 45	All 15 entries belonging to the following channel
46 through 50	The first five entries belonging to a third channel

If no later table exists within the allocation, the same writes proceed beyond the complete scantab allocation instead.

Two-player example

Suppose Player A owns 20 ships and occupies channel 0. Player B occupies channel 1.

Player A enters the game and findships() prints ships 1 through 20.
Ships 1 through 15 are stored in scantab[0] as intended.
Ship 16 is stored in scantab[1].ship[0].shipno.
Ships 17 through 20 replace the next four ship numbers in Player B's table.

If Player B is also choosing a persistent ship, Player A has replaced part of B's selection map. B can select a displayed number and have the database lookup fail, after which classic redraws the list and asks again. This is the observed failure in which simultaneous players can appear unable to select a ship.

If Player B is already playing, the same overwrite corrupts B's sensor table. An entry can retain an active flag and letter while its ship number now names an unrelated persistent ship. Later sensor and combat messages can lose an otherwise valid letter and print ?, associate an old letter with the wrong live slot, or use the corrupted value as though it were a valid live ship-array index. Those consequences overlap with Bug 20.

The corruption is timing-dependent. Player A's sixteenth selection may even appear to work if nothing changes the overwritten memory between rendering the list and entering 16. Another user's scan, login, or ship selection can replace that borrowed entry before A responds, making the same selection fail or resolve differently.

The selection read is also unbounded

Classic converts the entered number to a zero-based array index:

selection = atoi(margv[0]) - 1;
if (selection >= 0) {
        shpno = scantab[usrnum].ship[selection].shipno;
        /* load this user's record with that ship number */
}

It checks only the lower bound. Selection 16 reads the first entry in the next channel's table; selection 31 reads the first entry two tables later. An arbitrarily large positive number can read still farther beyond the requesting player's table.

The subsequent database lookup includes the requesting user's ID, so the ordinary result of reading another player's ship number is a failed lookup, not boarding that other player's ship. A coincidental ship-number match can select a different ship belonging to the requester. Regardless of the database result, the array access itself is outside its declared bounds.

Unbounded terminal output

Classic also appends every matching row to one formatted response and calls outprfge() only after the database walk. There is no row or page limit. A large fleet therefore produces a long terminal response before the selection prompt.

That presentation problem is secondary to the verified memory corruption. The cross-user failure comes from using SCANTAB past its fifteenth entry, not merely from terminal wrapping or a player entering a two-digit selection.

Correction in ge-next

ge-next 0.3 replaced the unbounded list with a page-based implementation. Each page contains at most NOSCANTAB, or 15, ships:

do {
        /* print and remember one ship on this page */
        sptr->ship[found].shipno = warsptr->shipno;
        ++found;
} while (qnxbtv() && found < NOSCANTAB);

The corrected flow:

counts the user's complete fleet separately from the current page;
clears all 15 page entries before filling a new page;
stops both output and storage after 15 ships;
offers N and P navigation when additional pages exist;
keeps the current page's persistent ship numbers in the channel's bounded table;
accepts numeric selections only from 1 through 15; and
rejects empty entries instead of attempting a database lookup with stale data.

The page is advanced through the ship database rather than by assuming that persistent ship numbers are consecutive. The correction therefore remains valid when ships have been destroyed and their identifiers contain gaps.

Because the list is now bounded, ge-next can safely support a configured fleet limit above 15. ge-next 0.3 raised the accepted maximum to 60 and was tested with multi-page fleets; the limit no longer describes the size of one temporary array.

Porting guidance

Do not use a gameplay cache as an unbounded menu buffer. Count the complete result set independently, render a bounded page, clear unused page entries, and validate both the lower and upper bounds of every selection. A persistent object identifier should still be loaded under the requesting user's identity rather than trusted solely because it appeared in temporary memory.

This defect is also a reminder that configuration limits must agree with the storage used by every consumer. A safe default does not make a larger advertised configuration safe.

Home · Play · Reference · Versions & history · Sysop · Development · Classic bugs