Skip to content

Automated code review: bounds-check & robustness findings (happy to send PRs) #47

Description

@grub-basket

Issue title: Automated code review — bounds-check & logic bugs (happy to send PRs)

Hi @jorio! Automated code review of Bugdom. No security review (old-game port — overkill). No PRs opened; glad to send focused PRs.

Prepared with Claude Fable 5 (Low effort mode). Manually sanity-checked; please review before merging.

Shared-engine findings

Same coordinated set I can PR across the Pangea ports (skeleton loader bounds, negative animNum, Collision.c overflow, Bones.c unchecked indices, Fences.c type > count off-by-one — the fence off-by-one is confirmed here too, src/Terrain/Fences.c).

Bugdom-specific

  • src/Enemies/Enemy_FireAnt.c:98 — if (gNumEnemyOfKind[ENEMY_KIND_FIREANT] > MAX_ENEMIES) compares a per-kind count against the global cap and uses > (every other enemy uses >= MAX_<KIND>); the real global guard is commented out above. The fire-ant limiter never rejects → a level with many fire-ant emitters spawns unbounded, exhausting the fixed pools. Fix: a real per-kind max.
  • src/Terrain/Fences.c:364 — letGoOver is initialized once before the fence loop and only ever set true (for wheat/forest fences), never reset per iteration. After hitting a walk-over fence, every later fence in the same call is treated as pass-over-able → player/dragonfly can clip through solid fences (order-dependent on fence indexing). Fix: reset letGoOver = false; at the top of the per-fence loop body.
  • src/Items/Liquids.c:321/306 — yTable[itemPtr->parm[2]] (6 elems) and yTable2[itemPtr->parm[2]] (8 elems) index with an unchecked asset Byte; the sibling AddLiquidPatch guards the identical pattern with GAME_ASSERT(parm[2] < 6). Fix: add the matching asserts.
  • src/Enemies/Enemy_QueenBee.c:272-283 — the base-search loop assumes queen-base IDs are exactly {0..N-1}; a map numbering bases 1..N (or with a gap) exits with b == gNumQueenBases and indexes gQueenBase[b] OOB (or a zero slot → world 0,0). Fix: if (b >= gNumQueenBases) b = 0;.
  • src/Items/Triggers.c:85-86 — gValveIsOpen[MAX_VALVE_IDS]/gDetonatorBlown[MAX_DETONATOR_IDS] are sized 255 but IDs come from asset parm bytes that can be 255 → 1-element OOB read/write (the two arrays clobber each other). Many call sites. Fix: size [256] or validate on read.
  • src/Enemies/Enemy_WorkerBee.c:432-434 (MoveWorkerBee_Pound) — passes pp.y+10 for both top and bottom of the collision box → degenerate zero-height box; the ball-mode pound attack misses. Fix: second arg should be pp.y-10.

Happy to PR any subset (the fence clip-through and fire-ant spawn cap are the most gameplay-visible).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions