Skip to content

[Issue Report]: Out-of-bounds write via unvalidated light ID in ChangeLightRadius() when loading save data #8546

@ACavalletto

Description

@ACavalletto

Operating System

Linux x86

DevilutionX version

1.5.5

Describe

The light ID loaded from save data is not properly validated before being used in ChangeLightRadius() (Source/lighting.cpp). The existing checks fail to reject IDs greater than or equal to MAXLIGHTS, allowing an out-of-range index to be used when accessing the global Lights array. A malformed or malicious save file can exploit this to cause an out-of-bounds write, leading to memory corruption, undefined behavior, or a crash.

To Reproduce

  1. Craft or modify a save file so that a light ID field holds a value ≥ MAXLIGHTS.
  2. Load the save file in DevilutionX.
  3. Trigger any code path that calls ChangeLightRadius() with the invalid ID.
  4. Observe a crash, memory corruption, or incorrect lighting behavior.

Expected Behavior

The game should perform a full bounds check on light IDs read from save data, ensuring the value is strictly less than MAXLIGHTS before accessing Lights[i]. Any ID that fails this check should be silently ignored rather than used.

Additional context

  • Vulnerable function: ChangeLightRadius() in Source/lighting.cpp
  • The root cause is an incomplete bounds check -- the upper bound (>= MAXLIGHTS) is not enforced, only a partial check is present.
  • Suggested fix: add an explicit guard such as if (id < 0 || id >= MAXLIGHTS) return; before any access to Lights[id].

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions