Skip to content

Swift: Add Command Injection query (CWE-078) #13726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 31, 2023

Conversation

maikypedia
Copy link
Contributor

WIP

@maikypedia maikypedia requested a review from a team as a code owner July 12, 2023 00:16
@maikypedia maikypedia changed the title WIP : Swift: Ruby: Add Command Injection query WIP : Swift: Add Command Injection query Jul 12, 2023
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Hi @maikypedia , this query looks great! You've marked it as a work-in-progress so I'll just make a few small comments for now. When you're ready we can either do a more detailed review (and add some tests) - or we can just merge your work it into an 'experimental' directory if you'd prefer us to take it from there.

maikypedia and others added 3 commits July 12, 2023 16:44
Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
@geoffw0
Copy link
Contributor

geoffw0 commented Jul 27, 2023

@maikypedia are you still working on this? Can I help with anything?

maikypedia and others added 2 commits July 27, 2023 22:45
Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>
@maikypedia
Copy link
Contributor Author

@maikypedia are you still working on this? Can I help with anything?

Hi @geoffw0 , the query is already finished, only the tests are missing but I am not familiar with writing stubs for swift 😅

@geoffw0
Copy link
Contributor

geoffw0 commented Jul 27, 2023

OK, please would you move the query from src/queries/Security/CWE-078 to src/experimental/Security/CWE-078 (this excludes it from the default code scanning query suite, for now). Then I can merge it, add the tests and any other necessary changes, and move it out of experimental when everything is ready.

Thanks!

PS: I can @mention you in the PR(s) I make if you want to follow along.

@maikypedia maikypedia changed the title WIP : Swift: Add Command Injection query Swift: Add Command Injection query Jul 27, 2023
@maikypedia maikypedia changed the title Swift: Add Command Injection query Swift: Add Command Injection query (CWE-078) Jul 27, 2023
@maikypedia
Copy link
Contributor Author

OK, please would you move the query from src/queries/Security/CWE-078 to src/experimental/Security/CWE-078 (this excludes it from the default code scanning query suite, for now). Then I can merge it, add the tests and any other necessary changes, and move it out of experimental when everything is ready.

Thanks!

PS: I can @mention you in the PR(s) I make if you want to follow along.

Done 😁

@github-actions
Copy link
Contributor

QHelp previews:

swift/ql/src/experimental/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 examples execute 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
}

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

task.launch()

References

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Thank you again for this contribution!

@geoffw0 geoffw0 merged commit e534afe into github:main Jul 31, 2023
@github github deleted a comment Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants