Skip to content

fix(exec-approvals): reject Allow rules with absolute file paths - #379

Merged
shanselman merged 2 commits into
openclaw:masterfrom
AlexAlves87:fix/exec-approvals-validator-path-allow
May 14, 2026
Merged

fix(exec-approvals): reject Allow rules with absolute file paths#379
shanselman merged 2 commits into
openclaw:masterfrom
AlexAlves87:fix/exec-approvals-validator-path-allow

Conversation

@AlexAlves87

Copy link
Copy Markdown
Contributor

Summary

ValidateExecApprovalRules accepted Allow rules whose pattern was an absolute file path (e.g. C:\Users\Public\evil.exe, \?\C:\evil.exe). A local process that obtained the MCP bearer token could use system.execApprovals.set to whitelist an attacker-controlled binary, then invoke it via system.run — a two-step local EoP.

The existing validator already blocked all-wildcard and dangerous-fragment patterns. This closes the gap on specific-path Allow rules.

Changes

  • SystemCapability.cs: add check in ValidateExecApprovalRules — rejects Allow patterns that start with a drive root (X:\, X:/) or a UNC/long-path prefix (\)
  • CapabilityTests.cs: add ExecApprovalsSet_RejectsAbsolutePathAllowRules theory covering drive-rooted, forward-slash, UNC, and long-path namespace patterns

Fixes #347.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com

ValidateExecApprovalRules accepted patterns like C:\evil.exe as Allow
rules. A local process with the MCP bearer token could use this to
whitelist an attacker-controlled binary, then invoke it via system.run
(two-step local EoP).

Add a check: Allow rules whose pattern starts with a drive root (X:\,
X:/) or a UNC/long-path prefix (\) are rejected. Legitimate rules
name commands, not paths.

Fixes openclaw#347.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@shanselman

Copy link
Copy Markdown
Collaborator

This is the right security direction, but the validator still appears bypassable with quoted absolute paths.

ValidateExecApprovalRules trims and lowercases the pattern, then checks normalized[0] for a drive letter or normalized.StartsWith(@"\\"). A pattern like "C:\Users\Public\evil.exe" starts with a quote, so it is accepted even though it still whitelists a specific absolute binary path. The same applies to quoted UNC/long-path forms.

Suggested fix: strip one layer of matching surrounding quotes before the absolute-path check, and add test cases for at least:

  • "C:\Users\Public\evil.exe"
  • 'C:\Users\Public\evil.exe'
  • "\\server\share\tool.exe"
  • "\\?\C:\evil.exe"

I would also consider forward-slash UNC/namespace forms if the command policy accepts them (//server/share/tool.exe, //?/C:/evil.exe).

The validator lowercased the pattern but did not strip surrounding quotes,
so "C:\evil.exe" and 'C:\evil.exe' bypassed the absolute-path rejection
added in the previous commit. Also add forward-slash UNC/namespace forms
(//server/share, //?/C:/evil.exe) to the check.

Strip one layer of matching " or ' before testing for drive-root, UNC,
and forward-slash UNC prefixes. Add quoted and forward-slash test cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@AlexAlves87

Copy link
Copy Markdown
Contributor Author

Thanks for catching the quoted-path bypass! Added quote stripping before the absolute-path check, and covered all the cases you mentioned — unquoted and quoted drive paths, UNC and long-path prefix forms, single-quoted variants, and the forward-slash equivalents.

@shanselman
shanselman merged commit 99e7440 into openclaw:master May 14, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[deep-review] C7: system.execApprovals.set validator accepts arbitrary file-path Allow rules

2 participants