Skip to content

Conversation

@bolinfest
Copy link
Collaborator

Previous to this PR, we used a hand-rolled PowerShell parser in windows_safe_commands.rs to take a &str of PowerShell script see if it is equivalent to a list of execvp(3) invocations, and if so, we then test each using is_safe_powershell_command() to determine if the overall command is safe:

/// Tokenizes an inline PowerShell script and delegates to the command splitter.
/// Examples of when this is called: pwsh.exe -Command '<script>' or pwsh.exe -Command:<script>
fn parse_powershell_script(script: &str) -> Option<Vec<Vec<String>>> {
let tokens = shlex_split(script)?;
split_into_commands(tokens)
}
/// Splits tokens into pipeline segments while ensuring no unsafe separators slip through.
/// e.g. Get-ChildItem | Measure-Object -> [['Get-ChildItem'], ['Measure-Object']]
fn split_into_commands(tokens: Vec<String>) -> Option<Vec<Vec<String>>> {

Unfortunately, our PowerShell parser did not recognize @(...) as a special construct, so it was treated as an ordinary token. This meant that the following would erroneously be considered "safe:"

ls @(calc.exe)

The fix introduced in this PR is to do something comparable what we do for Bash/Zsh, which is to use a "proper" parser to derive the list of execvp(3) calls. For Bash/Zsh, we rely on https://crates.io/crates/tree-sitter-bash, but there does not appear to be a crate of comparable quality for parsing PowerShell statically (https://github.com/airbus-cert/tree-sitter-powershell/ is the best thing I found).

Instead, in this PR, we use a PowerShell script to parse the input PowerShell program to produce the AST.

PowershellParseOutcome::Failed
}

fn powershell_candidates() -> &'static [&'static str] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use shell detection here?

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.

3 participants