Resolve vulnerability: Improper neutralization of special elements used in an OS Command ('OS Command Injection')

Placeholder Placeholder Noah Ingrequested to merge
remediate/improper-neutralization-of-special-elements-used-in-an-os-comma-D20251029T181555 into main

MR created from vulnerability: Improper neutralization of special elements used in an OS Command ('OS Command Injection')

AI GENERATED FIX

The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. Use this feature with caution. Before you run a pipeline or apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.

The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context.

Please see our documentation for more information about this feature.

Description:

The application was found with instances where user input is unsafely passed to the subprocess.run() or related functions, which can lead to command injection vulnerabilities. It specifically looks for cases where shell commands like sh, bash, etc. are executed with user-controlled input.

Command injection is a serious security vulnerability that allows an attacker to execute arbitrary system commands on the host operating system. This can lead to data breaches, data loss, system compromise, and other devastating impacts.

To fix command injection vulnerabilities, user input should never be passed directly to subprocess functions that execute system commands. Instead, use the subprocess module's argument list feature to pass command arguments safely without invoking a shell.

import subprocess

user_input = "file.txt"
subprocess.run(["ls", user_input])

General mitigation guidelines:

  • Never pass user input directly to subprocess functions that execute commands
  • Use the argument list feature of subprocess to pass command and arguments safely
  • Validate and sanitize any user input before using it in command execution

Summary:

Command Injection Vulnerability Fix

Vulnerability

The code contained a classic OS Command Injection vulnerability (CWE-78) where user input (customer_name) was directly concatenated into a shell command and executed using subprocess.run() with shell=True. This would allow an attacker to inject arbitrary commands by providing malicious input in the customer name field.

Fix

The fix addresses this vulnerability in two ways:

  1. Eliminated shell command for file writing:

    • Replaced the shell command that writes to a file with Python's native file I/O operations
    • Used with open(invoice_path, 'w') as f: f.write(invoice_content) to safely write the invoice content

  2. Removed shell=True and used argument list:

    • Changed the subprocess call to use an argument list instead of a string command
    • Removed the shell=True parameter which is a major security risk when combined with user input
    • Used subprocess.run(["echo", f"Invoice generated for {customer_name}"], ...) which prevents command injection

This approach follows the recommended security practice of never passing user input directly to shell commands and using the argument list feature of subprocess to safely handle command execution.

Identifiers:

  • A1:2017 - Injection
  • SAST Rules ID - python_exec_rule-subprocess-call
  • CWE-78
  • SAST Rules ID - python_exec_rule-subprocess-popen-shell-true
  • python-lang-cmdi-subprocess-taint
  • Bandit Test ID bandit.B604
  • Bandit Test ID bandit.B602
  • A03:2021 - Injection
  • Bandit Test ID bandit.B603
  • SAST Rules ID - python_exec_rule-subprocess-call-array
  • SAST Rules ID - python_exec_rule-subprocess-shell-TRUE
  • SAST Rules ID - python_exec_rule-start-process-path

Merge request reports