Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions report_generator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import subprocess

Choose a reason for hiding this comment

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

📋 Security Summary

This pull request introduces a new Python script for generating PDF reports. The script is vulnerable to command injection, which allows an attacker to execute arbitrary commands on the system.

🔍 General Feedback

  • The script uses user input directly in a shell command, which is a critical security risk.
  • The script should be updated to use shell=False and pass the command and its arguments as a list to subprocess.run.

Choose a reason for hiding this comment

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

Critical The script is vulnerable to command injection. You should import the shlex library to properly sanitize the user input.

Suggested change
import subprocess
import subprocess
import shlex


def generate_report(report_title, content_file):
"""Generates a PDF report with a user-supplied title."""


command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"

Choose a reason for hiding this comment

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

CRITICAL A command injection vulnerability exists in this line. The `report_title` variable, which is derived from user input, is directly embedded into a shell command. This allows an attacker to execute arbitrary commands by providing a malicious report title.
Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"
command = [
"pandoc",
content_file,
"-o",
"report.pdf",
"--metadata",
f"title={report_title}",
]

Choose a reason for hiding this comment

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

CRITICAL The report_title parameter, which is derived from user input, is directly used to construct a shell command. A malicious user can inject arbitrary commands by providing a crafted report title, leading to remote code execution.

Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"
import subprocess
import shlex
def generate_report(report_title, content_file):
"""Generates a PDF report with a user-supplied title."""
command = ["pandoc", content_file, "-o", "report.pdf", "--metadata", f"title={report_title}"]
result = subprocess.run(command, capture_output=True, text=True)
if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)
user_title = input("Enter report title: ")
generate_report(user_title, "report_data.md")

Choose a reason for hiding this comment

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

Critical The generate_report function is vulnerable to command injection. The report_title parameter, which is taken directly from user input, is used to construct a shell command that is executed with shell=True. A malicious user could provide a crafted title that includes arbitrary shell commands, which would then be executed on the system.

Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"
import subprocess
def generate_report(report_title, content_file):
"""Generates a PDF report with a user-supplied title."""
command = [
"pandoc",
content_file,
"-o",
"report.pdf",
"--metadata",
f"title={report_title}"
]
result = subprocess.run(command, capture_output=True, text=True)
if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)
user_title = input("Enter report title: ")
generate_report(user_title, "report_data.md")

Choose a reason for hiding this comment

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

CRITICAL: Command Injection

The script is vulnerable to command injection because it uses user-supplied input to construct a shell command that is then executed with shell=True. An attacker can provide a malicious report title to execute arbitrary commands on the system.

Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"
command = ["pandoc", content_file, "-o", "report.pdf", "--metadata", f"title={report_title}"]

Choose a reason for hiding this comment

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

CRITICAL: The generate_report function is vulnerable to command injection. The report_title parameter, which is taken directly from user input, is used to construct a shell command via an f-string. An attacker can provide a malicious title containing shell metacharacters to execute arbitrary commands on the system.

Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"
import shlex
def generate_report(report_title, content_file):
"""Generates a PDF report with a user-supplied title."""
command = ["pandoc", content_file, "-o", "report.pdf", "--metadata", f"title={report_title}"]
result = subprocess.run(command, capture_output=True, text=True)

Choose a reason for hiding this comment

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

🔴 Critical: This line constructs a shell command using raw user input, which creates a severe command injection vulnerability. A malicious title like '; rm -rf / could execute arbitrary commands. The command should be constructed as a list of arguments to be passed to subprocess.run safely.

Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"
command = [
"pandoc",
content_file,
"-o",
"report.pdf",
"--metadata",
f"title={report_title}",
]


Choose a reason for hiding this comment

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

Critical The script is vulnerable to command injection because it uses a user-supplied report_title to construct a shell command that is executed with shell=True. An attacker can inject arbitrary commands by crafting a malicious report title. You should use shlex.quote() to sanitize the user-supplied input.

Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{shlex.quote(report_title)}'"


result = subprocess.run(command, shell=True, capture_output=True, text=True)

Choose a reason for hiding this comment

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

🔴 The report_title variable, which is taken directly from user input, is used to construct a shell command. An attacker can inject arbitrary commands by crafting a malicious report title.

Suggested change
result = subprocess.run(command, shell=True, capture_output=True, text=True)
command = ["pandoc", content_file, "-o", "report.pdf", "--metadata", f"title={report_title}"]
result = subprocess.run(command, capture_output=True, text=True)

Choose a reason for hiding this comment

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

🔴 The script is vulnerable to command injection. The `report_title` variable, which is taken from user input, is directly embedded into a shell command. An attacker can provide a malicious title to execute arbitrary commands on the system.
    ```suggestion
    command = ["pandoc", content_file, "-o", "report.pdf", "--metadata", f"title={report_title}"]


    result = subprocess.run(command, capture_output=True, text=True)
    ```
    </COMMENT>

Choose a reason for hiding this comment

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

🔴 Critical: Executing a command with shell=True using unvalidated user input is extremely dangerous and leads to command injection. After modifying the command to be a list of arguments, you must remove shell=True to execute the command safely. Using check=True is also recommended to automatically raise an exception if the command fails.

Suggested change
result = subprocess.run(command, shell=True, capture_output=True, text=True)
result = subprocess.run(command, capture_output=True, text=True)


Comment on lines +7 to +11

Choose a reason for hiding this comment

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

🔴 The use of shell=True with user-provided input in the subprocess.run function creates a command injection vulnerability. A malicious user could provide a crafted report title to execute arbitrary commands on the system.

To fix this, you should pass the command as a list of arguments to subprocess.run and avoid using shell=True. This ensures that user input is treated as a single argument and not interpreted by the shell.

Suggested change
command = f"pandoc {content_file} -o report.pdf --metadata title='{report_title}'"
result = subprocess.run(command, shell=True, capture_output=True, text=True)
command = [
"pandoc",
content_file,
"-o",
"report.pdf",
"--metadata",
f"title={report_title}",
]
result = subprocess.run(command, capture_output=True, text=True, check=True)

if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)

user_title = input("Enter report title: ")
generate_report(user_title, "report_data.md")
Loading