[Coding Guideline]: Prevent OS Command Injection

[sei-dsvoboda]

Chapter

Program Structure and Compilation

Guideline Title

Prevent OS Command Injection

Category

Mandatory

Status

Draft

Release Begin

1.0.0

Release End

latest

FLS Paragraph ID

fls_hdwwrsyunir

Decidability

Undecidable

Scope

Module

Tags

injection,sanitization

Amplification

Commands that are passed to an external OS command interpreter, like std::process::Command, should not allow untrusted input to be parsed as part of the command syntax.

Instead, an untrusted input should be passed as a single argument.

Exception(s)

No response

Rationale

This rule was inspired by CERT-J-IDS07.

When preparing a command to be executed by the operating system, untrusted input should be sanitized to make sure it does not alter the syntax of the command to be executed. For commands that do not tokenize their arguments, such as sh, the easiest way to do this is to avoid mixing untrusted data with trusted data via concatenation or formatting (a la format!()). Instead provide the untrusted data as a lone argument. The Command::new() constructor makes this easy by accepting the pre-tokenized arguments as a list of strings.

Traditionally untrusted data should be one argument (aka command-line token). OS command injection occurs when a malicious data fools the command tokenizer into interpreting it as multiple arguments, or even multiple commands. Complexity in the command tokenizer can exacerbate this problem, leading to vulnerabilities such as CVE-2024-24576. See RUST-WIN-ARG-SPLIT and SEI-BATBADBUT for more information.

Non-Compliant Example 1 - Prose

The following code lists the contents the directory provided in the dir variable. However, since this variable is untrusted, a dir such as dummy | echo BOO will cause the command to be executed. Thus, the program prints "BOO".

Non-Compliant Example 1 - Code

use std::process::{Command, Output};
use std::io;

fn files(dir: &str) -> io::Result<Output> {
    return Command::new("sh")
        .arg("-c")
        .arg(format!("ls {dir}"))
        .output();
}

fn main() {
    if cfg!(unix) {
        let _ = files("dummy | echo BOO");  // Program prints "BOO"
    }
}

Non-Compliant Example 2 - Prose (Optional)

No response

Non-Compliant Example 2 - Code (Optional)

No response

Non-Compliant Example 3 - Prose (Optional)

No response

Non-Compliant Example 3 - Code (Optional)

No response

Non-Compliant Example 4 - Prose (Optional)

No response

Non-Compliant Example 4 - Code (Optional)

No response

Compliant Example 1 - Prose

An untrusted input should be passed as a single argument. This prevents any spaces or other shell punctuation in the input from being misinterpreted by the OS command interpreter.

Compliant Example 1 - Code

use std::process::{Command, Output};
use std::io;

fn files(dir: &str) -> io::Result<Output> {
    return Command::new("ls")
        .arg(dir)
        .output();
}

fn main() {
    if cfg!(unix) {
        let _ = files("dummy | echo BOO");  // Command is invalid, but does not print BOO
    }
}

Compliant Example 2 - Prose (Optional)

A better approach is to avoid OS commands and use a specific API (in this case fs::read_dir()) to achieve the desired result.

Compliant Example 2 - Code (Optional)

use std::fs;
use std::io;

fn files(dir: &str) -> io::Result<Vec<std::ffi::OsString>> {
    return fs::read_dir(dir)?
        .map(|res| res.map(|e| e.file_name()))
        .collect();
}

fn main() {
    if cfg!(unix) {
        let _ = files("dummy | echo BOO");  // Command is invalid, but does not print BOO
    }
}

Compliant Example 3 - Prose (Optional)

No response

Compliant Example 3 - Code (Optional)

No response

Compliant Example 4 - Prose (Optional)

No response

Compliant Example 4 - Code (Optional)

No response

Bibliography

[github-actions]

👋 Hey @squell! You've been assigned to review this coding guideline issue.

Your Role as Reviewer

As outlined in our contribution guide, please:

1. Provide initial feedback within 14 days

2. Work with @sei-dsvoboda to flesh out the concept and ensure the guideline is well-prepared for a Pull Request

3. Check the prerequisites before the issue is ready to become a PR:

   • The new rule isn't already covered by another rule
   • All sections contain some content
   • Content written may be incomplete, but must not be incorrect
   • The 🧪 Code Example Test Results section shows all example code compiles

4. When ready, add the sign-off: create pr from issue label to signal the contributor should create a PR

Bot Commands

If you need to pass this review:

• @guidelines-bot /pass [reason] - Pass just this issue to the next reviewer
• @guidelines-bot /away YYYY-MM-DD [reason] - Step away from the queue until a date
• @guidelines-bot /release [@username] [reason] - Release assignment (yours or someone else's with triage+ permission)

To assign someone else:

• @guidelines-bot /r? @username - Assign a specific reviewer
• @guidelines-bot /r? producers - Request the next reviewer from the queue

Other commands:

• @guidelines-bot /claim - Claim this review for yourself
• @guidelines-bot /label +label-name - Add a label
• @guidelines-bot /label -label-name - Remove a label
• @guidelines-bot /queue - Show reviewer queue
• @guidelines-bot /commands - Show all available commands

[github-actions]

📋 RST Preview for Coding Guideline

This is an automatically generated preview of your coding guideline in reStructuredText format.

📁 Target Location

Create a new file: src/coding-guidelines/program-structure-and-compilation/gui_a3PpM90Fppwh.rst.inc

Note: The .rst.inc extension prevents Sphinx from auto-discovering the file.
It will be included via the chapter's index.rst.

We add it to this path, to allow the newly added guideline to appear in the correct chapter.

🗂️ Update Chapter Index

Update src/coding-guidelines/program-structure-and-compilation/index.rst to include gui_a3PpM90Fppwh.rst.inc, like so:

Chapter Name Here <- chapter heading inside of `src/coding-guidelines/program-structure-and-compilation/index.rst`
=================

.. include:: gui_7y0GAMmtMhch.rst.inc -| existing guidelines
.. include:: gui_ADHABsmK9FXz.rst.inc  |
...                                    |
...                                    |
.. include:: gui_RHvQj8BHlz9b.rst.inc  |
.. include:: gui_dCquvqE1csI3.rst.inc -|
.. include:: gui_a3PpM90Fppwh.rst.inc <- your new guideline to add

🧪 Code Example Test Results

✅ All 3 code example(s) compiled successfully!

Note: Code examples are tested with rustc --edition=2021.
Hidden lines (prefixed with # ) are included in compilation.
Examples without a fn main() are automatically wrapped.

📚 Bibliography & Citations

✅ 4 bibliography entry/entries validated successfully!

• 4 citation reference(s) found and matched

Citation Summary:

• ✅ Matched references: [CERT-J-IDS07], [CVE-2024-24576], [RUST-WIN-ARG-SPLIT], [SEI-BATBADBUT]

Parsed entries:

Citation Key	Author	Title	URL	Used?
[CERT-J-IDS07]	SEI CERT Java	IDS07-J. Sanitize untrusted data pass...	Link	✅
[RUST-WIN-ARG-SPLIT]	Module process	Windows Argument Splitting	Link	✅
[SEI-BATBADBUT]	SEI Blog	What Recent Vulnerabilities Mean to R...	Link	✅
[CVE-2024-24576]	MITRE	CVE-2024-24576	Link	✅

📝 How to Use This

1. Fork the repository (if you haven't already) and clone it locally
2. Create a new branch from main:

   git checkout main
   git pull origin main
   git checkout -b guideline/your-descriptive-branch-name

3. Create the guideline file:

   mkdir -p src/coding-guidelines/program-structure-and-compilation

4. Copy the RST content below into a new file named gui_a3PpM90Fppwh.rst.inc
5. Update the chapter index - Add an include directive to src/coding-guidelines/program-structure-and-compilation/index.rst:

   .. include:: gui_a3PpM90Fppwh.rst.inc

   Keep the includes in alphabetical order by guideline ID.
6. Build locally to verify the guideline renders correctly:

   ./make.py

7. Commit and push your changes:

   git add src/coding-guidelines/program-structure-and-compilation/
   git commit -m "Add guideline: <your guideline title>"
   git push origin guideline/your-descriptive-branch-name

8. Open a Pull Request against main

📄 Click to expand RST content

.. SPDX-License-Identifier: MIT OR Apache-2.0
   SPDX-FileCopyrightText: The Coding Guidelines Subcommittee Contributors

.. guideline:: Prevent OS Command Injection
    :id: gui_a3PpM90Fppwh
    :category: mandatory
    :status: draft
    :release: 1.0.0-latest
    :fls: fls_hdwwrsyunir
    :decidability: undecidable
    :scope: module
    :tags: injection,sanitization

    Commands that are passed to an external OS command interpreter, like ``std::process::Command``, should not allow untrusted input to be parsed as part of the command syntax.

    Instead, an untrusted input should be passed as a single argument.

    .. rationale::
        :id: rat_IaAZISFOmAt0
        :status: draft

        This rule was inspired by :cite:`gui_a3PpM90Fppwh:CERT-J-IDS07`.

        When preparing a command to be executed by the operating system, untrusted input should be sanitized to make sure it does not alter the syntax of the command to be executed. For commands that do not tokenize their arguments, such as ``sh``, the easiest way to do this is to avoid mixing untrusted data with trusted data via concatenation or formatting (a la ``format!()``). Instead provide the untrusted data as a lone argument. The ``Command::new()`` constructor makes this easy by accepting the pre-tokenized arguments as a list of strings.

        Traditionally untrusted data should be one argument (aka command-line token). OS command injection occurs when a malicious data fools the command tokenizer into interpreting it as multiple arguments, or even multiple commands. Complexity in the command tokenizer can exacerbate this problem, leading to vulnerabilities such as :cite:`gui_a3PpM90Fppwh:CVE-2024-24576`. See :cite:`gui_a3PpM90Fppwh:RUST-WIN-ARG-SPLIT` and :cite:`gui_a3PpM90Fppwh:SEI-BATBADBUT` for more information.

    .. non_compliant_example::
        :id: non_compl_ex_Owe2nVInv90z
        :status: draft

        The following code lists the contents the directory provided in the ``dir`` variable. However, since this variable is untrusted, a ``dir`` such as ``dummy | echo BOO`` will cause the command to be executed. Thus, the program prints “BOO”.

        .. rust-example::

            use std::process::{Command, Output};
            use std::io;

            fn files(dir: &str) -> io::Result<Output> {
                return Command::new("sh")
                    .arg("-c")
                    .arg(format!("ls {dir}"))
                    .output();
            }

            fn main() {
                if cfg!(unix) {
                    let _ = files("dummy | echo BOO");  // Program prints "BOO"
                }
            }


    .. compliant_example::
        :id: compl_ex_rJeLKhdopITN
        :status: draft

        An untrusted input should be passed as a single argument. This prevents any spaces or other shell punctuation in the input from being misinterpreted by the OS command interpreter.

        .. rust-example::

            use std::process::{Command, Output};
            use std::io;

            fn files(dir: &str) -> io::Result<Output> {
                return Command::new("ls")
                    .arg(dir)
                    .output();
            }

            fn main() {
                if cfg!(unix) {
                    let _ = files("dummy | echo BOO");  // Command is invalid, but does not print BOO
                }
            }


    .. compliant_example::
        :id: compl_ex_BSjAFOLfL4Rk
        :status: draft

        A better approach is to avoid OS commands and use a specific API (in this case ``fs::read_dir()``) to achieve the desired result.

        .. rust-example::

            use std::fs;
            use std::io;

            fn files(dir: &str) -> io::Result<Vec<std::ffi::OsString>> {
                return fs::read_dir(dir)?
                    .map(|res| res.map(|e| e.file_name()))
                    .collect();
            }

            fn main() {
                if cfg!(unix) {
                    let _ = files("dummy | echo BOO");  // Command is invalid, but does not print BOO
                }
            }


    .. bibliography::
        :id: bib_CNrst9CcDVQJ
        :status: draft

        .. list-table::
           :header-rows: 0
           :widths: auto
           :class: bibliography-table

          * - :bibentry:`gui_a3PpM90Fppwh:CERT-J-IDS07`
            - SEI CERT Java. "IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method." https://wiki.sei.cmu.edu/confluence/x/xTdGBQ
          * - :bibentry:`gui_a3PpM90Fppwh:RUST-WIN-ARG-SPLIT`
            - Module process. "Windows Argument Splitting." https://doc.rust-lang.org/std/process/index.html#windows-argument-splitting
          * - :bibentry:`gui_a3PpM90Fppwh:SEI-BATBADBUT`
            - SEI Blog. "What Recent Vulnerabilities Mean to Rust | “BatBadBut” Command Injection with Windows’ cmd.exe (CVE-2024-24576)." https://www.sei.cmu.edu/blog/what-recent-vulnerabilities-mean-to-rust/
          * - :bibentry:`gui_a3PpM90Fppwh:CVE-2024-24576`
            - MITRE. "CVE-2024-24576." https://nvd.nist.gov/vuln/detail/CVE-2024-24576

---

_{🤖 This comment was automatically generated from the issue content. It will be updated when you edit the issue body.}

[felix91gr]

@guidelines-bot /claim

[github-actions]

✅ @felix91gr has claimed this review.

[sei-dsvoboda]

Is it worthwhile to cite the 'BatBadBut' vulnerability, which is somewhat relevant? Cite: https://www.sei.cmu.edu/blog/what-recent-vulnerabilities-mean-to-rust/