security(AcceptInput): sanitize player names to prevent server command injection#18
security(AcceptInput): sanitize player names to prevent server command injection#18
Conversation
…names - Added SanitizePlayerName to strip ';', '"' and newlines from commands. - This prevents players from executing arbitrary server commands by changing their Steam name (e.g., "name; quit"). - Applied sanitization to both !activator.name and direct VScript strings.
There was a problem hiding this comment.
Pull request overview
This PR addresses a critical RCE vulnerability (issue #17) where players could execute arbitrary server commands by including control characters (semicolons, quotes, newlines) in their Steam names. The fix implements sanitization at two levels: global sanitization of all say/say_team commands and targeted sanitization when replacing !activator.name placeholders.
Changes:
- Added
SanitizeSayCommandandSanitizePlayerNamefunctions to strip dangerous characters (;,",\n,\r) from player names - Optimized placeholder replacement to only retrieve client data when placeholders are present in commands
- Fixed format string vulnerability by using
ServerCommand("%s", sCommand)instead ofServerCommand(sCommand)
| bool SanitizeSayCommand(char[] sCommand, int maxlen) | ||
| { | ||
| // Only sanitize chat commands | ||
| if (strncmp(sCommand, "say ", 4, false) != 0 && strncmp(sCommand, "say_team ", 9, false) != 0) | ||
| return false; | ||
|
|
||
| return SanitizePlayerName(sCommand, maxlen); | ||
| } |
There was a problem hiding this comment.
The SanitizeSayCommand function is only called in AcceptInput (line 199), but not in the Detour_SendToServerConsole or Detour_SetValue functions. This means that if VScript uses SendToConsole("say " + playerName + " message"), the command will not be sanitized for command injection. While the PR description claims "global protection" for VScript string concatenation, this only applies to commands going through point_servercommand, not through SendToConsole. Consider adding the same sanitization to Detour_SendToServerConsole to ensure consistent protection across all command sources.
Description
This PR addresses a critical security vulnerability where players could execute arbitrary server commands by including control characters (like
;,", or newlines) in their Steam names.The Issue
#17
Impact
This fix is global. It protects all maps on the server, including those that inject names directly via VScript string concatenation and those using the plugin's built-in placeholders.
Testing Performed
Player123) still display correctly in chat.Lardy;kill a) are sanitized toLardy kill aand do not trigger secondary commands.Name" say hi) do not break thesaycommand structure.!activator.nameplaceholders are correctly sanitized before execution.Checklist
!activatortags).Special thanks to
LardyfromNiDE Z.Escapeserver for reporting this RCE.