Jump to content

Classic bug: Planet trade-password terminator overrun

From Galactic Empire wiki
Bug ID GE-BUG-038
Affected versions Classic 3.2e and earlier versions derived from the same source
Status Verified in source; corrected in maintained descendants
Fixed in Elwynor's 32-bit port; ge-next 0.3
Primary source Planet trade-password handling in mnu_admenu2i(); GALPLNT.password layout

Summary

Classic GE writes the terminating null byte for a planet trade password one byte beyond the end of its password[10] array. In the classic planet record layout, that byte is the first byte of the immediately following lastattack field.

The routine also copies only nine password characters and leaves password[9] untouched. Nine-character or longer input is therefore terminated correctly only when that old byte already happens to contain zero.

The adjacent field appears to be an abandoned feature and is not read by any published classic 3.2e routine. That limits the visible consequences in a normal game, but the operation is still an out-of-bounds write that is saved back into the planet database.

Planet record layout

Classic stores each planet in a fixed 512-byte GALPLNT record. The relevant fields are consecutive: 

char warnings;
char password[10];
char lastattack[UIDSIZ];
ITEM items[NUMITEMS];

In the 16-bit record layout, password occupies byte offsets 91 through 100. lastattack begins at offset 101. Character arrays do not require alignment padding between them, so password[10] and lastattack[0] designate the same stored byte in the expected layout: 

offset  91 ... 100  101 ... 130
field   password     lastattack
index   [0] ... [9]  [0] ... [29]
                     ^
                     password[10] writes here

An index equal to an array's declared length is outside that array. The final valid element of password[10] is password[9].

Faulty password assignment

After the planet owner selects the trade-password option in ADMIN, mnu_admenu2i() executes: 

strncpy(plptr->password, margv[0], 9);
plptr->password[10] = 0;

These two constants do not describe one coherent buffer operation:

  • strncpy(..., 9) writes only elements 0 through 8.
  • Element 9 is not explicitly changed by either statement.
  • Element 10 is outside the array and receives the null terminator instead.

For input shorter than nine characters, strncpy() pads the copied range with null bytes, so the resulting password is terminated within elements 0 through 8. For input nine characters or longer, all nine copied bytes are nonzero and the routine depends on the pre-existing value in element 9.

If password[9] is zero, the apparent password is the expected nine-character truncation. This is common because newly created planet records are cleared before receiving the default password none. It is not, however, guaranteed by this assignment.

If password[9] contains a nonzero byte, string operations continue through it and stop at the zero incorrectly written into lastattack[0]. The effective password then consists of the nine new characters followed by one stale character.

Adjacent-field corruption

The out-of-bounds store clears lastattack[0]. A conventional null-terminated string whose first byte is zero is empty, so any user ID previously stored there would be erased when the planet owner changed the trade password. The modified planet record is then written to the database, making the corruption persistent.

Despite its name and comment, lastattack is not part of a working classic feature. In the published 3.2e source it is initialized when a planet is created or reset, but no routine records an attacker there or reads it for gameplay. Consequently, clearing it does not normally change combat, ownership, or notifications. The bug is more likely to appear as a malformed full-length password than as a visible failure involving the adjacent field.

This limited consequence is accidental. A change to the structure or an implementation that made use of lastattack would turn the same off-by-one store into active state corruption.

Example

Assume the ten password bytes contain the following before the owner enters a new value. The old password is still the valid string none, but byte 9 contains a stale X after its earlier terminator: 

index:      0 1 2 3 4 5 6 7 8 9
value:      n o n e \0 \0 \0 \0 \0 X

The owner enters marketkey, which contains nine characters. Classic copies those characters into indexes 0 through 8 but does not change the old X at index 9: 

password:   marketkeyX
index:      0123456789

It then writes a zero to index 10, actually clearing lastattack[0]. Functions that display or compare the password see marketkeyX, not marketkey. A player supplying the nine-character value selected by the owner can therefore be refused trading or maintenance access.

Fresh classic records normally have zero in index 9, which is why this symptom is not inevitable. The example shows what the code permits when that byte is nonzero, including in edited, migrated, or previously corrupted records.

Corrections

Elwynor's 32-bit classic port retains password[10] and the original nine-character limit, but corrects the terminator index: 

strncpy(plptr->password, margv[0], 9);
plptr->password[9] = '\0';

This guarantees termination without changing the classic record layout.

The initial ge-next correction, made during 0.3 development, expanded the field to password[11], allowing ten password characters plus a terminator. At that stage it replaced the unused remainder of the old lastattack field with temporary padding. This preserved the offsets of every following field in the then-current structure: 

char password[11];
char filler2[UIDSIZ - 1];

That was an intermediate compatibility step, not the final ge-next 0.3 record design. The later structure reorganization removed the unused warnings and lastattack fields, removed filler2, and placed the item array directly after the enlarged password: 

int taxrate;
char password[11];
ITEM items[NUMITEMS];

The reorganization also corrected the structures to their real database record sizes and consolidated unused space into padding at the end. The current 16-bit GALPLNT is the actual 493-byte MajorBBS record; its password begins at offset 90 and its item array at offset 101. ge-next32 uses the same logical field order, with the alignment required by its platform and padding to its 512-byte MajorBBS V10 record.

The assignment derives both the copy length and terminator position from the actual destination size: 

strncpy(plptr->password, margv[0], sizeof(plptr->password) - 1);
plptr->password[sizeof(plptr->password) - 1] = 0;

This removes the mismatched magic numbers, always terminates the string, and supports the same ten-character maximum used for team passwords. ge-next32 uses the same corrected assignment and the restructured field layout. These changes were all completed before ge-next 0.3 was released; that release's larger database redesign required a new game rather than conversion of classic planet records.

Retrieved from "https://manicpop.org/gewiki/index.php?title=Classic_bug:_Planet_trade-password_terminator_overrun&oldid=1568"