fix: sanitize source slug + replace shell execSync with execFileSync (command injection) - #22
Open
jgaddis99 wants to merge 1 commit into
Open
Conversation
…duler calls schedule_job's `source` parameter was concatenated into the job slug without going through slugify(), allowing crafted quote/shell syntax to break out of the double-quoted execSync launchctl/systemctl command strings and execute arbitrary same-user commands. Two-layer fix: 1. Run args.source through slugify() so the slug is always [a-z0-9-] only, closing the injection at the point of input. 2. Replace all execSync shell-string calls for launchctl and systemctl with execFileSync + argument array, eliminating shell interpretation entirely regardless of slug content (defense in depth). Reported as TKT-1651, auto-promoted from security anomaly scout.
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
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.
Security fix — command injection via unsanitized
sourceparameterVulnerability
schedule_job'ssourceparameter was concatenated into the job slug without sanitization:The slug flows into
plistPath/legacyPlistPath, which are then shell-interpolated:A crafted
sourcevalue such asevil"$(touch /tmp/pwned)"xbreaks out of the double-quoted string and executes arbitrary commands as the current user whenschedule_jobis called.The same class of issue affects the systemctl calls on Linux —
job.slugis interpolated intoexecSyncshell strings without shell escaping.Fix
Layer 1 — sanitize at input (line 2585): run
args.sourcethroughslugify()so the slug is always[a-z0-9-]only.Layer 2 — remove the shell entirely: replace every
execSync(template-string)for launchctl and systemctl calls withexecFileSync(binary, args[]). No shell spawned, no injection surface regardless of what slug content looks like.execFileSyncwas already imported fromchild_process; no new dependencies.Affected calls fixed
installLaunchdJob: 3execSynclaunchctl calls →execFileSyncuninstallLaunchdJob: 1execSynclaunchctl call →execFileSyncinstallSystemdJob: 5execSyncsystemctl calls →execFileSyncuninstallSystemdJob: 3execSyncsystemctl calls →execFileSyncisSystemdUserAvailable: 1execSyncsystemctl call →execFileSyncschedule_job execute:args.sourcenow passes throughslugify()before slug construction