fix(security): prevent RCON command injection in player kick/ban/unban (HIGH)
Player id and ban reason flowed unsanitized into the single-line RCON command,
so a control char (newline/CR) in 'reason' could break the framing and inject a
second console command — an RBAC-escalation vector (a Moderator-role user could
run arbitrary RCON via the ban reason field).
- validate player id against a safe token charset /^[A-Za-z0-9_.:-]{1,64}$/ and
reject otherwise (multi-game safe — not a Rust-only SteamID64 regex, so
Conan/Funcom and Dune ids still pass)
- strip C0 control chars from reason, collapse whitespace, cap at 200 chars
- coerce ban duration to a non-negative integer
Flagged by automated commit security review. Backend tsc green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,4 @@
|
|||||||
import { Injectable } from '@nestjs/common';
|
import { Injectable, BadRequestException } from '@nestjs/common';
|
||||||
import { InjectRepository } from '@nestjs/typeorm';
|
import { InjectRepository } from '@nestjs/typeorm';
|
||||||
import { Repository } from 'typeorm';
|
import { Repository } from 'typeorm';
|
||||||
import { PlayerAction } from '../../entities/player-action.entity';
|
import { PlayerAction } from '../../entities/player-action.entity';
|
||||||
@@ -142,14 +142,32 @@ export class PlayersService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private buildRconCommand(dto: PlayerActionDto): string {
|
private buildRconCommand(dto: PlayerActionDto): string {
|
||||||
|
// Defense-in-depth against RCON command injection. The command is a single
|
||||||
|
// line; an id or reason containing a newline/control char could break the
|
||||||
|
// framing and inject a second console command. So:
|
||||||
|
// - the player id must be a safe token (no whitespace/control chars) — a
|
||||||
|
// permissive charset, not a Rust-only SteamID64 regex, so Conan (Funcom)
|
||||||
|
// and Dune ids still validate. Reject outright if not.
|
||||||
|
// - the free-text reason has control chars stripped and is length-capped.
|
||||||
|
// - duration is coerced to a non-negative integer.
|
||||||
|
const id = dto.steam_id ?? '';
|
||||||
|
if (!/^[A-Za-z0-9_.:-]{1,64}$/.test(id)) {
|
||||||
|
throw new BadRequestException('Invalid player id');
|
||||||
|
}
|
||||||
|
const safeReason =
|
||||||
|
(dto.reason ?? 'banned').replace(/[\u0000-\u001F]+/g, ' ').replace(/\s+/g, ' ').trim().slice(0, 200) || 'banned';
|
||||||
|
const secs = Number.isFinite(dto.duration_minutes)
|
||||||
|
? Math.max(0, Math.floor((dto.duration_minutes as number) * 60))
|
||||||
|
: 0;
|
||||||
|
|
||||||
switch (dto.action_type) {
|
switch (dto.action_type) {
|
||||||
case 'kick':
|
case 'kick':
|
||||||
return `kick ${dto.steam_id}${dto.reason ? ' ' + dto.reason : ''}`;
|
return `kick ${id}${dto.reason ? ' ' + safeReason : ''}`;
|
||||||
case 'ban':
|
case 'ban':
|
||||||
// banid <steamId> <reason> <durationSeconds> — 0 = permanent
|
// banid <steamId> <reason> <durationSeconds> — 0 = permanent
|
||||||
return `banid ${dto.steam_id} ${dto.reason ?? 'banned'} ${dto.duration_minutes ? dto.duration_minutes * 60 : 0}`;
|
return `banid ${id} ${safeReason} ${secs}`;
|
||||||
case 'unban':
|
case 'unban':
|
||||||
return `unban ${dto.steam_id}`;
|
return `unban ${id}`;
|
||||||
default:
|
default:
|
||||||
return '';
|
return '';
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user