Skip to content

Harden SQL input handling and fix unban log consistency in KnockbackRestrict#51

Open
Rushaway wants to merge 9 commits into
mainfrom
escape
Open

Harden SQL input handling and fix unban log consistency in KnockbackRestrict#51
Rushaway wants to merge 9 commits into
mainfrom
escape

Conversation

@Rushaway
Copy link
Copy Markdown
Member

Summary

This PR improves SQL safety across the plugin by escaping all user-influenced string inputs before query formatting, and fixes inconsistencies in unban logging behavior.

What Changed

  • Updated Kban_PublishKunban to actually use the unban reason in KbRestrict_srvlogs.message:
    • From static message: Removed Kban
    • To: Removed Kban (Reason: %s) (escaped)
  • Added missing SQL escaping in multiple query paths for sensitive fields:
    • client_steamid
    • client_ip
    • admin_steamid
    • map
    • log message
  • Hardened VerifyKbanClient by escaping SteamID/IP before SELECT and UPDATE queries.
  • Hardened Kban_AddBan and Kban_AddOfflineBan by escaping additional DB-bound fields, not only names/reason.
  • Hardened OnKbanAdded lookup query by escaping SteamID/IP in WHERE.
  • Fixed offline unban update path in helpers/menus.sp:
    • Corrected reason_removed assignment.
    • Added missing WHERE clause to avoid broad updates.
    • Escaped admin SteamID, reason, target SteamID, and IP before query use.

Why

  • Some DB queries previously used dynamic strings without explicit escaping.
  • One menu-based unban query had an incomplete UPDATE statement and no WHERE, which was risky.
  • Kban_PublishKunban escaped reason but did not use it in SQL logs, creating a logic mismatch.

Impact

  • Improves protection against malformed or malicious input in SQL statements.
  • Preserves existing functionality while making logs more informative and consistent.
  • Reduces risk of accidental mass updates in offline unban flow.

Rushaway added 5 commits May 18, 2026 21:41
Enhanced Kban_RemoveBan function to escape additional parameters for admin Steam ID, reason, and client IP.
Add IP escaping for database update to prevent SQL injection.
Copilot AI review requested due to automatic review settings May 18, 2026 19:46
@Rushaway
Copy link
Copy Markdown
Member Author

@Dolly132

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens SQL input handling in the KnockbackRestrict SourceMod plugin by expanding DB string escaping across several query paths, and it improves unban log consistency by including the unban reason in server logs.

Changes:

  • Escapes additional DB-bound fields (SteamID, IP, map, message) in multiple query/insert/update paths to reduce SQL injection/malformed input risk.
  • Updates unban logging (KbRestrict_srvlogs.message) to include the (escaped) unban reason.
  • Fixes the offline unban menu UPDATE to correctly set reason_removed and adds a missing WHERE clause to prevent broad updates.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
addons/sourcemod/scripting/KnockbackRestrict.sp Adds escaping for more query inputs; includes unban reason in srvlogs; hardens verification and post-insert lookups.
addons/sourcemod/scripting/include/KnockbackRestrict.inc Bumps patch version from 4.0.7 to 4.0.8.
addons/sourcemod/scripting/helpers/menus.sp Fixes offline unban UPDATE correctness (reason assignment + WHERE) and escapes added parameters.

Comment thread addons/sourcemod/scripting/KnockbackRestrict.sp
Comment thread addons/sourcemod/scripting/KnockbackRestrict.sp Outdated
Rushaway added 4 commits May 21, 2026 16:21
Removed unnecessary escaping of SteamID and IP in queries. Simplified database queries by directly using the SteamID and IP from the g_sSteamIDs and g_sIPs arrays.
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.

2 participants