fix: harden path traversal protection and block privilege escalation commands
- Add sanitizeCommand() to strip null bytes and URL-decode (%2e%2e%2f) before guard checks, preventing encoded traversal bypass - Refactor handleCdCommand() to use tools.ValidatePath() instead of fragile strings.HasPrefix, fixing workspace-suffix confusion and adding symlink escape protection - Block su, doas, pkexec in defaultDenyPatterns (sudo was already blocked) - Add 6 test cases covering URL-encoded traversal, null bytes, symlink escape, and privilege escalation command blocking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
9b65e77d22
commit
4d4f33fa56
4 changed files with 147 additions and 11 deletions
|
|
@ -1397,7 +1397,8 @@ func (al *AgentLoop) executeCmdMode(
|
|||
}
|
||||
|
||||
// handleCdCommand handles the cd command in command mode, updating per-session working directory.
|
||||
// Special paths (cd, cd ~, cd /, cd /xxx) are redirected to the workspace directory for safety.
|
||||
// Special paths (cd, cd ~, cd /) are redirected to the workspace directory for safety.
|
||||
// Uses tools.ValidatePath for robust containment: filepath.Rel + filepath.IsLocal + symlink resolution.
|
||||
func (al *AgentLoop) handleCdCommand(content, sessionKey string, agent *AgentInstance) string {
|
||||
parts := strings.Fields(content)
|
||||
workspace := agent.Workspace
|
||||
|
|
@ -1408,15 +1409,13 @@ func (al *AgentLoop) handleCdCommand(content, sessionKey string, agent *AgentIns
|
|||
target = workspace
|
||||
} else {
|
||||
target = parts[1]
|
||||
// Strip null bytes (defense-in-depth against bypass attempts)
|
||||
target = strings.ReplaceAll(target, "\x00", "")
|
||||
// Expand ~ prefix: treat ~ as workspace root (not $HOME)
|
||||
if strings.HasPrefix(target, "~/") {
|
||||
target = workspace + target[1:]
|
||||
}
|
||||
// Absolute paths (e.g. cd /etc) → redirect to workspace
|
||||
if filepath.IsAbs(target) {
|
||||
target = workspace
|
||||
}
|
||||
// Resolve relative paths
|
||||
// Resolve relative paths against session working dir
|
||||
if !filepath.IsAbs(target) {
|
||||
currentDir := al.getSessionWorkDir(sessionKey)
|
||||
if currentDir == "" {
|
||||
|
|
@ -1426,11 +1425,13 @@ func (al *AgentLoop) handleCdCommand(content, sessionKey string, agent *AgentIns
|
|||
}
|
||||
}
|
||||
|
||||
target = filepath.Clean(target)
|
||||
|
||||
// Prevent traversal outside workspace via ../
|
||||
if !strings.HasPrefix(target, workspace) {
|
||||
// Validate path is within workspace (handles traversal, symlinks, prefix matching correctly)
|
||||
validated, err := tools.ValidatePath(target, workspace, true)
|
||||
if err != nil {
|
||||
// Path escapes workspace — fall back to workspace root
|
||||
target = workspace
|
||||
} else {
|
||||
target = validated
|
||||
}
|
||||
|
||||
info, err := os.Stat(target)
|
||||
|
|
|
|||
|
|
@ -669,6 +669,71 @@ func TestHandleCdCommand_TraversalBlocked(t *testing.T) {
|
|||
_ = result
|
||||
}
|
||||
|
||||
// TestHandleCdCommand_NullByte verifies that null bytes in cd target
|
||||
// are stripped and cannot bypass workspace restriction.
|
||||
func TestHandleCdCommand_NullByte(t *testing.T) {
|
||||
workspace := t.TempDir()
|
||||
cfg := &config.Config{
|
||||
Agents: config.AgentsConfig{
|
||||
Defaults: config.AgentDefaults{
|
||||
Workspace: workspace,
|
||||
Model: "test-model",
|
||||
MaxTokens: 4096,
|
||||
},
|
||||
},
|
||||
}
|
||||
msgBus := bus.NewMessageBus()
|
||||
provider := &mockProvider{}
|
||||
al := NewAgentLoop(cfg, msgBus, provider)
|
||||
agent := al.registry.GetDefaultAgent()
|
||||
|
||||
al.setSessionWorkDir("test", workspace)
|
||||
|
||||
// Null byte in path should be stripped — traversal caught after stripping
|
||||
al.handleCdCommand("cd sub\x00dir/../../..", "test", agent)
|
||||
workDir := al.getSessionWorkDir("test")
|
||||
|
||||
if workDir != workspace {
|
||||
t.Errorf("Expected workDir=%s after null-byte cd, got %s", workspace, workDir)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleCdCommand_SymlinkEscape verifies that a symlink inside workspace
|
||||
// pointing outside is blocked by ValidatePath's symlink resolution.
|
||||
func TestHandleCdCommand_SymlinkEscape(t *testing.T) {
|
||||
workspace := t.TempDir()
|
||||
outsideDir := t.TempDir()
|
||||
|
||||
// Create a symlink inside workspace pointing outside
|
||||
symlink := filepath.Join(workspace, "escape")
|
||||
if err := os.Symlink(outsideDir, symlink); err != nil {
|
||||
t.Skipf("Cannot create symlink: %v", err)
|
||||
}
|
||||
|
||||
cfg := &config.Config{
|
||||
Agents: config.AgentsConfig{
|
||||
Defaults: config.AgentDefaults{
|
||||
Workspace: workspace,
|
||||
Model: "test-model",
|
||||
MaxTokens: 4096,
|
||||
},
|
||||
},
|
||||
}
|
||||
msgBus := bus.NewMessageBus()
|
||||
provider := &mockProvider{}
|
||||
al := NewAgentLoop(cfg, msgBus, provider)
|
||||
agent := al.registry.GetDefaultAgent()
|
||||
|
||||
al.setSessionWorkDir("test", workspace)
|
||||
|
||||
al.handleCdCommand("cd escape", "test", agent)
|
||||
workDir := al.getSessionWorkDir("test")
|
||||
|
||||
if workDir != workspace {
|
||||
t.Errorf("Expected symlink escape to be blocked, workDir=%s, workspace=%s", workDir, workspace)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleExtensionCommand_EmojiPassthrough verifies that emoji-like
|
||||
// messages starting with : are not intercepted as commands.
|
||||
func TestHandleExtensionCommand_EmojiPassthrough(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import (
|
|||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
|
|
@ -48,6 +49,9 @@ var defaultDenyPatterns = []*regexp.Regexp{
|
|||
regexp.MustCompile(`\$\(\s*wget\s+`),
|
||||
regexp.MustCompile(`\$\(\s*which\s+`),
|
||||
regexp.MustCompile(`\bsudo\b`),
|
||||
regexp.MustCompile(`\bsu\b`),
|
||||
regexp.MustCompile(`\bdoas\b`),
|
||||
regexp.MustCompile(`\bpkexec\b`),
|
||||
regexp.MustCompile(`\bchmod\s+[0-7]{3,4}\b`),
|
||||
regexp.MustCompile(`\bchown\b`),
|
||||
regexp.MustCompile(`\bpkill\b`),
|
||||
|
|
@ -255,8 +259,18 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *ToolResult
|
|||
}
|
||||
}
|
||||
|
||||
// sanitizeCommand strips null bytes and decodes URL-encoded sequences
|
||||
// so that encoded traversal patterns (e.g. %2e%2e%2f) are detected by guards.
|
||||
func sanitizeCommand(cmd string) string {
|
||||
cmd = strings.ReplaceAll(cmd, "\x00", "")
|
||||
if decoded, err := url.PathUnescape(cmd); err == nil {
|
||||
cmd = decoded
|
||||
}
|
||||
return cmd
|
||||
}
|
||||
|
||||
func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||
cmd := strings.TrimSpace(command)
|
||||
cmd := sanitizeCommand(strings.TrimSpace(command))
|
||||
lower := strings.ToLower(cmd)
|
||||
|
||||
for _, pattern := range t.denyPatterns {
|
||||
|
|
|
|||
|
|
@ -296,3 +296,59 @@ func TestGuardCommand_DotSlashExecutable(t *testing.T) {
|
|||
t.Errorf("Expected output 'ok', got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGuardCommand_URLEncodedTraversal verifies that URL-encoded path traversal
|
||||
// sequences (%2e%2e%2f → ../) are detected and blocked.
|
||||
func TestGuardCommand_URLEncodedTraversal(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
tool := NewExecTool(tmpDir, true)
|
||||
tool.SetRestrictToWorkspace(true)
|
||||
|
||||
msg := tool.guardCommand("cat %2e%2e%2f%2e%2e%2fetc/passwd", tmpDir)
|
||||
if msg == "" {
|
||||
t.Error("Expected URL-encoded path traversal to be blocked")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGuardCommand_NullByte verifies that null bytes in commands are stripped
|
||||
// before guard checks so they cannot bypass traversal detection.
|
||||
func TestGuardCommand_NullByte(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
tool := NewExecTool(tmpDir, true)
|
||||
tool.SetRestrictToWorkspace(true)
|
||||
|
||||
msg := tool.guardCommand("cat foo\x00../../etc/passwd", tmpDir)
|
||||
if msg == "" {
|
||||
t.Error("Expected null-byte traversal to be blocked")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGuardCommand_SuBlocked verifies that su and related privilege
|
||||
// escalation commands are blocked by deny patterns.
|
||||
func TestGuardCommand_SuBlocked(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
tool := NewExecTool(tmpDir, true)
|
||||
|
||||
cases := []string{"su", "su -", "su root", "doas ls", "pkexec /bin/bash"}
|
||||
for _, cmd := range cases {
|
||||
msg := tool.guardCommand(cmd, tmpDir)
|
||||
if msg == "" {
|
||||
t.Errorf("Expected %q to be blocked", cmd)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestGuardCommand_SuNoFalsePositive verifies that words containing "su"
|
||||
// as a substring are NOT blocked (e.g. summary, result, surplus).
|
||||
func TestGuardCommand_SuNoFalsePositive(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
tool := NewExecTool(tmpDir, false) // no workspace restriction for this test
|
||||
|
||||
cases := []string{"echo surplus", "cat summary.txt", "ls result/"}
|
||||
for _, cmd := range cases {
|
||||
msg := tool.guardCommand(cmd, tmpDir)
|
||||
if msg != "" {
|
||||
t.Errorf("Expected %q to NOT be blocked, got: %s", cmd, msg)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue