GHSA-7fq5-7wr8-rjwj: OliveTin has a Concurrent Template Parsing Race Condition which Leads to Cross-Request Command Contamination
SummaryOliveTin's template engine uses a **single shared `text/template.Template` instance** (`tpl` package-level variable in `service/internal/tpl/templates.go`) across all goroutines. Every action execution calls `tpl.Parse(source)` followed by `t.Execute()` on this shared instance with no synchronization. When two or more actions execute concurrently (which is the normal case — each `ExecRequest` spawns a goroutine), a race condition occurs: one goroutine's `Parse` overwrites the template tree while another goroutine is calling `Execute`, causing:**Cross-user command contamination**: User A's arguments rendered in User B's shell command template**Go runtime panic**: Concurrent map writes in Go's `text/template` internal structures cause a fatal crash**Incorrect command execution**: Template/argument mismatch produces unexpected or dangerous shell commandsCWECWE-362 (Concurrent Execution using Shared Resource with Improper Synchronization)CWE-567 (Unsynchronized Access to Shared Data in a Multithreaded Context)Affected VersionsAll versions (the shared template has existed since the template system was introduced)DetailsThe Shared Template InstanceIn `service/internal/tpl/templates.go`:This is a **package-level variable** — a single `*template.Template` shared across the entire process.Unsafe Parse + Execute PatternThe `parseTemplate` function is called for every template rendering:**Critical**: `tpl.Parse(source)` returns the same pointer as `tpl` (Go's `template.Parse` modifies the receiver and returns it). So `t` and `tpl` are the **same object**. When two goroutines call `parseTemplate` concurrently:If the goroutines interleave:Goroutine A would execute `rm -rf {{ .Arguments.path }}` with `dataA` — which either errors (missing key) or, if `dataA` happens to have a `path` argument, executes with an unintended value.No Synchronization ExistsA search for any synchronization primitives in the `tpl` package confirms **zero mutex, lock, or atomic operations**:Concurrent Goroutine ConfirmationIn `service/internal/executor/executor.go`, `ExecRequest` launches each action in a new goroutine:The execution chain includes `stepParseArgs`, which calls `ParseTemplateWithActionContext`, which calls `parseTemplate`. Multiple concurrent action executions will race on the shared `tpl` variable.Go Runtime Crash VectorGo's `text/template.Parse` internally modifies the template's `common` struct, which contains a `tmpl map[string]*Template`. In Go, concurrent map writes cause an **unrecoverable fatal error**:This is not a panic that can be recovered — it terminates the entire process. Two concurrent `Parse` calls can trigger this, crashing OliveTin.Template Contamination VectorEven without a crash, the race can produce dangerous results:**User A** triggers action: `shell: "echo Hello {{ .Arguments.name }}"` with `name=Alice`**User B** triggers action: `shell: "sudo systemctl restart {{ .Arguments.service }}"` with `service=nginx`Race occurs: User A's `Execute` runs on User B's parsed templateIf User A's arguments contain a `service` key, that value is substituted into `sudo systemctl restart {{ .Arguments.service }}`If User A's arguments do NOT contain `service`, `missingkey=error` causes an error — but only AFTER the template was already partially evaluatedCall ChainPoCPrerequisitesOliveTin instance with at least 2 configured actionsAbility to trigger concurrent action executionsConfigStep 1: Trigger concurrent executionsStep 2: Check for crashStep 3: Check logs for contaminationPython PoC — Race TriggerGo Race Detector VerificationIf you can run OliveTin with Go's race detector enabled:Expected output:Impact**Process Crash (DoS)**: Concurrent map writes in Go cause an unrecoverable `fatal error`, crashing the entire OliveTin service**Cross-User Command Contamination**: User A's arguments may be rendered in User B's shell command template, potentially executing commands with wrong/dangerous arguments**Privilege Escalation via Contamination**: If a low-privilege user's arguments contaminate a high-privilege action's template, the result could be unintended command execution**Data Leakage**: Arguments (which may contain secrets like passwords) could be rendered in another user's action outputRemediation**Create a new template per parse call** instead of reusing the package-level singleton:**Alternative**: Use `template.Must(tpl.Clone())` to create a thread-safe copy per call:**Alternative**: Add a mutex around `parseTemplate` (but this serializes all template rendering and hurts performance):Option 1 (new template per call) is the recommended fix — it's simple, safe, and has negligible performance impact.ResourcesGo `text/template` documentation: "A Template's Parse method must not be called concurrently"CWE-362: Concurrent Execution using Shared Resource with Improper Synchronization`service/internal/tpl/templates.go` — shared `tpl` variable and `parseTemplate` function`service/internal/executor/executor.go` — `ExecRequest` goroutine launch (line ~524)
CVSS v3.1: 7.5