[geoffw0]

Adds a unified/swift/command-line-injection query for unified Swift. I got Copilot to do this step-by-step, first copying files from the original Swift implementation, then updating the structure, porting the QL itself, and finally fixing a couple of issues. The original tests are preserved and test annotations updated so we can see where we stand. Tiny bit of manual fixup afterwards.

I'll share a skill to do all this when it's sufficiently mature / proven.

The query only finds a couple of the test cases right now. I'm pretty sure we're missing some relevant summary models, and there might be other issues e.g. with data flow. But it's time to share what I have and open it up to discussion.

TODO: review DCA run.

[github-actions]

QHelp previews:

unified/ql/src/queries/security/CWE-078/CommandInjection.qhelp

System command built from user-controlled sources

Constructing a system command with unsanitized user input is dangerous, since a malicious user may be able to craft input that executes arbitrary code.

Recommendation

If possible, use hard-coded string literals to specify the command to run. Instead of interpreting user input directly as command names, examine the input and then choose among hard-coded string literals.

If this is not possible, then add sanitization code to verify that the user input is safe before using it.

Example

The following example executes code from user input without sanitizing it first:

var task = Process()
task.launchPath = "/bin/bash"
task.arguments = ["-c", userControlledString] // BAD

task.launch()

If user input is used to construct a command it should be checked first. This ensures that the user cannot insert characters that have special meanings:

func validateCommand(_ command: String) -> String? {
    let allowedCommands = ["ls -l", "pwd", "echo"]
    if allowedCommands.contains(command) {
        return command
    }
    return nil
}

if let validatedString = validateCommand(userControlledString) {
    var task = Process()
    task.launchPath = "/bin/bash"
    task.arguments = ["-c", validatedString] // GOOD

    task.launch()
}

References

• OWASP: Command Injection.
• Common Weakness Enumeration: CWE-78.
• Common Weakness Enumeration: CWE-88.

[geoffw0]

@asgerf Copilot added this near-duplicate of the model below, and it does indeed let us catch an additional test case where the default argument value for terminationHandler is used. But I suspect we might prefer a general fix for default arguments in the logic that matches models with function calls?

[geoffw0]

See: https://developer.apple.com/documentation/foundation/process/run(_:arguments:terminationhandler:)

[asgerf]

Yes, I'm aware of the limitation and expect to make a general fix for this. But it's not a small fix.