Skip to content

Fix missing OpenFile() check and unbounded TextColors write - #25

Merged
Rushaway merged 2 commits into
masterfrom
fix/config-load-safety
Sep 21, 2026
Merged

Rushaway merged 2 commits into
masterfrom
fix/config-load-safety

Conversation

@Rushaway

Copy link
Copy Markdown
Member

Summary

  • RefreshConfig() never checked OpenFile()'s result before reading from it.
  • CountColors was never bounded against MAXTEXTCOLORS, so a config with more than 100 color entries would write past the end of TextColors.

Fixes #24

Test plan

  • Compiled cleanly via the local SourcePawn compiler, no errors/warnings.

🤖 Generated with Claude Code

RefreshConfig() never checked whether OpenFile() succeeded before reading
from the handle, and never bounded CountColors against the fixed-size
TextColors array (MAXTEXTCOLORS = 100), so a config with more than 100
color entries would trip a runtime array-bounds error mid-reload.

Fixes #24

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 20, 2026 12:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Reload failures must be propagated and reported instead of being reported as successful.

Review effort: Lite
Findings: None

What changed in this PR

Fixes configuration loading by validating OpenFile() and bounding color entries to MAXTEXTCOLORS.

Changes:

  • Handles missing or unreadable configuration files.
  • Prevents out-of-bounds writes.
  • Logs ignored excess entries.
File Summary
addons/​sourcemod/​scripting/​SMTextReplace.sp Adds file-open validation and bounds protection during configuration loading.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Rushaway
Rushaway merged commit 641a86b into master Sep 21, 2026
6 checks passed
@Rushaway
Rushaway deleted the fix/config-load-safety branch September 21, 2026 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RefreshConfig() doesn't check OpenFile() result, and can write past TextColors' bounds

2 participants