fix: pause-permission tags, role clobber, env parsing, jointeam guard, constant-time token (#552, #553, #558)#164
Open
Flegma wants to merge 2 commits into
Open
fix: pause-permission tags, role clobber, env parsing, jointeam guard, constant-time token (#552, #553, #558)#164Flegma wants to merge 2 commits into
Flegma wants to merge 2 commits into
Conversation
…nt-time token
- TimeoutSystem: match the clan tags actually assigned on connect ([admin] /
[organizer]); the old [administrator]/[match_organizer]/[tournament_organizer]
strings never matched, so privileged pause/resume never worked (#552).
- InitConnect: the role assignment used two ifs, so an Administrator's 'admin'
tag was immediately clobbered to 'organizer' by the else. Made it
if/else-if/else (#558).
- InitConnect: verify the connect token with CryptographicOperations.
FixedTimeEquals instead of string != (constant-time) (#558).
- PlayerConnected: guard the jointeam team-number arg with int.TryParse so a
non-numeric client argument can't throw out of the hook (#558).
- swiftly EnvironmentService: split each .env line on the first '=' only. The
previous Split('=', RemoveEmptyEntries) dropped any value containing '='
(base64 secrets, tokens, connection strings) or any empty value (#553).
counterstrikesharp + swiftly build 0/0; swiftly suite 80/80.
…ollow-up)
- EnvironmentService: skip a line whose key trims to empty (e.g. ' =value'),
which would otherwise throw ArgumentException from SetEnvironmentVariable('').
Load() has no surrounding try/catch, so that would crash plugin init.
- swiftly TimeoutSystem: the same never-matching clan-tag check
([administrator]/[match_organizer]/[tournament_organizer]) exists in the
SwiftlyS2 port; align it to the assigned [admin]/[organizer] tags in both
spots so pause/resume works there too when swiftly becomes the runtime.
swiftly build 0/0, suite 80/80.
Contributor
Author
|
Independent code review run on the diff. Two follow-ups in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Self-contained game-server fixes from the 2026-07 audit. Closes #552, #553, #558.
Changes
Pause/resume permission clan tags (#552)
TimeoutSystem.CanPause/CanResumecomparedplayer.Clanto[administrator],[match_organizer],[tournament_organizer], but connect assigns[admin]/[streamer]/[organizer](PlayerConnected.cssetsplayer.Clan = "[{role}]"from InitConnect's role map). The strings never matched, so admin/organizer pause and resume never worked. Now checks[admin]/[organizer].InitConnect role assignment clobber (#558)
The role map used
if (Administrator) {...}immediately followed byif (Streamer) {...} else {...}, so an Administrator (not a Streamer) fell into theelseand gotorganizer, overwriting theadmintag set one line above. Changed toif / else if / else. (This is what made the admin tag exist for the #552 fix to match.)Constant-time connect-token check (#558)
InitConnect verified the HMAC connect token with
computedToken != password(string compare, short-circuits on first mismatch). Switched toCryptographicOperations.FixedTimeEquals.jointeam argument guard (#558)
HandleJoinTeamdidint.Parse(info.ArgByIndex(1))on a client command argument, which throws on non-numeric input. Guarded withint.TryParse(returnsHookResult.Continueon bad input).swiftly .env parsing (#553)
EnvironmentServicesplit each line withSplit('=', RemoveEmptyEntries)and skipped anything not exactly two parts, silently dropping every value that contains=(base64 secrets, tokens, connection strings) or is empty. Now splits on the first=only, and skips blanks/comments.Testing
dotnet build(Release): counterstrikesharp (net8) and swiftly (net10) both 0 errors / 0 warnings.Deferred from this PR
#559 (packaging): the swiftly
setup.shif $VAR = truequoting bug and theDockerfile.devvsDockerfilemetamod/CSS version skew. Lower value; the version alignment wants a deliberate choice of the target versions rather than a blind bump. Left for a follow-up.