Truncated registry keys can collide #54

Open
opened 2026-08-02 18:33:40 -04:00 by tachikoma · 0 comments
Collaborator

Source: TODO.md, "Truncated registry keys can collide" (at bbb7b8f)

Found while turning -Wall on.

akgl_actor_initialize and akgl_character_initialize document their name fields
as "truncated, not rejected, if the source name is longer", and that is what they
do -- the copy is aksl_strncpy bounded to the field, so it always terminates
now. Termination was the overread half, and it is fixed.

The other half is not: the truncated name is the registry key. Two distinct
200-character names truncate to the same 127-byte key, and the second
SDL_SetPointerProperty silently replaces the first. The objects are different;
the registry cannot tell.

The same applies to akgl_Sprite::name (128), akgl_SpriteSheet::name (512) and
the tilemap's object and tileset names.

Whether that matters depends on whether long asset names are realistic, and 127
bytes is generous for a hand-written name in a JSON file -- which is why this is
recorded rather than already fixed.

The fix is a contract change: refuse an over-long name with
AKERR_OUTOFBOUNDS instead of truncating. Every one of those headers currently
promises the opposite. aksl_strncpy already reports exactly that status when the
bytes do not fit, so the change is to stop capping n at size - 1 and let it
raise.

Related but distinct: actors and characters unregister under a different key
than they register, filed separately. That one is a disagreement between two
sites; this one is a property of truncation itself.

Files: src/actor.c, src/character.c, src/sprite.c, src/tilemap.c, and the headers that document truncation


Filed by Tachikoma (Claude Code, Opus 5, 1M context)

**Source:** TODO.md, "Truncated registry keys can collide" (at bbb7b8f) Found while turning `-Wall` on. `akgl_actor_initialize` and `akgl_character_initialize` document their name fields as "truncated, not rejected, if the source name is longer", and that is what they do -- the copy is `aksl_strncpy` bounded to the field, so it always terminates now. **Termination was the overread half, and it is fixed.** The other half is not: **the truncated name is the registry key.** Two distinct 200-character names truncate to the same 127-byte key, and the second `SDL_SetPointerProperty` silently replaces the first. The objects are different; the registry cannot tell. The same applies to `akgl_Sprite::name` (128), `akgl_SpriteSheet::name` (512) and the tilemap's object and tileset names. Whether that matters depends on whether long asset names are realistic, and 127 bytes is generous for a hand-written name in a JSON file -- which is why this is recorded rather than already fixed. **The fix is a contract change:** refuse an over-long name with `AKERR_OUTOFBOUNDS` instead of truncating. Every one of those headers currently promises the opposite. `aksl_strncpy` already reports exactly that status when the bytes do not fit, so the change is to stop capping `n` at `size - 1` and let it raise. Related but distinct: actors and characters *unregister* under a different key than they register, filed separately. That one is a disagreement between two sites; this one is a property of truncation itself. **Files:** `src/actor.c`, `src/character.c`, `src/sprite.c`, `src/tilemap.c`, and the headers that document truncation --- Filed by Tachikoma (Claude Code, Opus 5, 1M context)
tachikoma added this to the 0.10.0 milestone 2026-08-02 18:33:40 -04:00
tachikoma added the defectdesign-decisionabi-breakblast-radius:medium labels 2026-08-02 18:33:40 -04:00
tachikoma added the status::grooming label 2026-08-02 18:49:24 -04:00
Sign in to join this conversation.