Skip to content

Clamp sprite limit inputs to prevent OOB and exhaustion#124

Merged
segrax merged 1 commit into
masterfrom
codex/propose-fix-for-sprite-vector-oob-write
May 24, 2026
Merged

Clamp sprite limit inputs to prevent OOB and exhaustion#124
segrax merged 1 commit into
masterfrom
codex/propose-fix-for-sprite-vector-oob-write

Conversation

@segrax
Copy link
Copy Markdown
Member

@segrax segrax commented May 24, 2026

Motivation

  • The code made mSpritesMax externally controllable (CLI/INI/demo JSON) while many engine paths still assume at least 45 sprites, allowing out-of-bounds writes and memory-exhaustion via crafted inputs.\
  • Introduce a minimal, centralized validation to prevent malicious or accidental underflow/overflow of the sprite vector while preserving original defaults.

Description

  • Added MIN_SPRITES_MAX = 45 and MAX_SPRITES_MAX = 100000 constants to sFodderParameters in Source/Parameters.hpp to centralize safe bounds.\
  • Tied CUSTOM_DEFAULT_MAX_SPRITES to the new MAX_SPRITES_MAX to keep custom-mode defaults consistent in Source/Parameters.cpp.\
  • Clamped mSpritesMax to the [MIN_SPRITES_MAX, MAX_SPRITES_MAX] range when loading from demo/save JSON in sFodderParameters::FromJson.\
  • Clamped mSpritesMax after parsing CLI (--max-sprite) in sFodderParameters::ProcessCLI and after reading engine.maxsprite from openfodder.ini in sFodderParameters::ProcessINI to cover all untrusted input paths.\

Testing

  • No unit tests were executed for this change.\
  • Performed automated code searches with rg to verify all input code paths (mSpritesMax, --max-sprite, Sprite_Clear_All, Map_Load_Sprites) are covered by the clamping changes and inspected diffs with git diff, which succeeded.\
  • Changes were committed successfully with the message Clamp max sprite parameter to safe range.

Codex Task

@segrax segrax merged commit 85fca26 into master May 24, 2026
1 of 6 checks passed
@segrax segrax deleted the codex/propose-fix-for-sprite-vector-oob-write branch May 24, 2026 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant