Validate bundleId/subsystem filter to prevent predicate injection in log capture#407
Conversation
…ection in log capture User-supplied bundleId and custom subsystem filter values are interpolated into NSPredicate strings passed to `log stream --predicate`. A bundleId containing double quotes or other special characters could inject arbitrary predicate syntax, altering log filtering behavior (information disclosure from other subsystems, or denial of service via malformed predicates). Add validation against a strict allowlist pattern (alphanumeric, dots, hyphens, underscores) before any string interpolation into predicates. Invalid values are rejected early with a descriptive error message. CWE-78 / predicate injection mitigation.
1ddc844 to
383d45e
Compare
commit: |
|
Thanks @sebastiondev for the contribution here. The underlying issue is valid, and the original fix was a good direction. I rebased the PR onto the latest What changed during reconciliation:
One related regression became clear while resolving this: the current simulator logging path no longer exposes the older configurable subsystem filter behavior. I opened #409 to track restoring a safe public filter option, with the same validation requirements this PR establishes. Validation run locally after the rebase:
All passed. |
|
Thanks for reviewing. @lewiswigmore |
Summary
startLogCaptureinsrc/utils/log_capture.tsinterpolates the caller-suppliedbundleId(and array entries ofsubsystemFilter) directly into the NSPredicate string passed toxcrun simctl spawn <udid> log stream --predicate .... A bundleId containing a double quote can break out of the quoted predicate value and broaden the filter to capture log output from other subsystems — other apps, or Apple system frameworks — into the log file the caller can subsequently read back.This is best classified as CWE-74 / CWE-77 (improper neutralization in a downstream component) rather than CWE-78: the predicate string is delivered as a single argv element, so no shell is involved, but the predicate grammar that
log streamparses internally still treats"as a string delimiter regardless of how the argv arrived.Affected component
src/utils/log_capture.tsstartLogCapture,buildPredicatemainat the time of writing (Release v2.3.0, commitb7f8a89). The same shape exists in the v2.5.x line undersrc/utils/simulator-steps.ts::launchSimulatorAppWithLogging; the fix should be ported there too.Data flow
bundleIdas a parameter — controlled by the MCP client. In agentic deployments (LLM tool use over untrusted content like a README, web page, or repo file) the LLM can be steered into supplying a hostile value.startLogCapture(params, ...)destructuresbundleIdwith no validation.buildPredicateconstructs, for the default'app'filter, a string of the formsubsystem == "${bundleId}".xcrun simctl spawn <udid> log stream --style json --predicate <predicate>.bundleIdis also used in the on-disk log file path, so a../variant relocates writes.Proof of concept
With:
the predicate becomes:
and
log streamwrites Safari's OSLog output into the file the caller will later read. Any subsystem emitting sensitive data (auth flows, URLs, tokens, MDM, keychain access logging) becomes readable. Backslashes and additional quote sequences allow analogous broadening; entries insidesubsystemFilter: string[]have the same issue.Fix
Validate
bundleIdand any customsubsystemFilterstrings against a strict allowlist (^[A-Za-z0-9._-]+$) at the entrypoint ofstartLogCapture, before they reach the predicate or path construction. Apple bundle identifiers and OSLog subsystem names use only those characters in practice, so the allowlist has no false-positive impact on legitimate use.When validation fails, the function returns the standard
{ error }shape used elsewhere in the module — noxcrunprocess is spawned and no log file is created.Tests
The patch adds four tests to
src/utils/__tests__/log_capture.test.ts:subsystemFilterarray entries containing double quotesEach rejection case asserts both that an error is returned and that no executor calls were made (the spawn never happens). The full suite passes locally.
Adversarial review
Before submitting, we tried to disprove this. The argv-array spawn pattern does prevent shell metacharacter injection — but it does not protect the predicate grammar parsed by
log stream, which treats"as a string delimiter regardless of how the argv arrived. There is no upstream validation ofbundleIdin the tool entrypoint, and the same value is used to derive the on-disk log file path, so a path-traversal variant is also reachable. The preconditions to exploit (issuing a tool call with a chosen bundleId) do not already grant the attacker access to other apps' OSLog output, so the impact is not subsumed by trust already placed in the tool.Disclosure note
SECURITY.mddirects reports to GitHub Private Vulnerability Reporting, but PVR is currently disabled on the repository (/private-vulnerability-reportingreturns{"enabled": false}) and non-maintainers cannot create a repository advisory. Given the medium severity, the public-source nature of the issue, and that the fix is a straightforward input allowlist, we're submitting the fix as a regular PR. Happy to redirect to a private channel if one is enabled — just let us know.Scope
The diff touches only
src/utils/log_capture.ts(+33) and its test file (+76); no behavior change for valid inputs.Submitted by Sebastion — autonomous open-source security research from Foundation Machines. Free for public repos via the Sebastion AI GitHub App.