Skip to content
Closed
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 It's good practice to add a shebang line to the top of your Python scripts, especially if they are meant to be executable.

Suggested change
import subprocess
#!/usr/bin/env python3
import subprocess


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

Choose a reason for hiding this comment

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

🟡 The function generate_report currently prints the result to the console. It would be more reusable if it returned a status or the result object, and let the caller decide what to print.

Suggested change
"""Generates a PDF report with a user-supplied title."""
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}'"
return subprocess.run(command, shell=True, capture_output=True, text=True)
user_title = input("Enter report title: ")
result = generate_report(user_title, "report_data.md")
if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)



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

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. The generate_report function uses a user-supplied title to construct a shell command without proper sanitization. A malicious user can inject arbitrary commands by crafting a malicious title, leading to remote code execution.

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}"]
result = subprocess.run(command, capture_output=True, text=True)

Copy link
Copy Markdown

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 derived from user input, is directly used to construct a shell command that is executed with subprocess.run and shell=True. A malicious user can provide a specially crafted 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}"]
result = subprocess.run(command, capture_output=True, text=True)

Copy link
Copy Markdown

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 derived from user input, is directly used to construct a shell command that is executed with shell=True. A malicious user can inject arbitrary commands by providing a crafted 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}"]
result = subprocess.run(command, capture_output=True, text=True)


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The use of an f-string to construct a shell command with user-provided input is highly insecure and can lead to command injection vulnerabilities. An attacker could provide a malicious report title that includes shell commands, which would then be executed by subprocess.run.

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 = [
"pandoc",
content_file,
"-o",
"report.pdf",
"--metadata",
f"title={report_title}",
]

Copy link
Copy Markdown

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 derived from user input, is directly used to construct a shell command that is executed by subprocess.run with shell=True. A malicious user could provide a crafted report title to execute arbitrary commands on the system.

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

Copy link
Copy Markdown

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 is passed directly into the shell command. An attacker could provide a malicious title like '; rm -rf / to execute arbitrary commands.

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

Copy link
Copy Markdown

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 is taken from user input and directly formatted into the shell command. A malicious user could provide a title that includes shell commands, which would then be executed.

To fix this, you should pass the command as a list of arguments to subprocess.run and avoid shell=True. Also, you should sanitize the report_title to prevent any malicious input.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Critical: This code is vulnerable to command injection. The report_title is taken from user input and directly used to construct a shell command. An attacker could provide a malicious report title to execute arbitrary commands on the system.

Suggested change
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='{shlex.quote(report_title)}'"
result = subprocess.run(command, shell=True, capture_output=True, text=True)
if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)


result = subprocess.run(command, shell=True, capture_output=True, text=True)
Comment on lines +7 to +10
Copy link
Copy Markdown

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 an f-string to build the command creates a command injection vulnerability. A malicious user could provide a report title that includes arbitrary shell commands.

To fix this, you should pass the command as a list of arguments to subprocess.run and remove 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)

Copy link
Copy Markdown

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 derived from user input, is directly embedded into a shell command. An attacker can provide a malicious title containing shell metacharacters (e.g., ;, &&, ||, $(...)) to execute arbitrary commands on the system where this script is run.

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)

Comment on lines +7 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Critical Security Vulnerability: Command Injection

The use of shell=True with a command string constructed from raw user input (report_title) allows for arbitrary command injection. A malicious user could provide a title like '; rm -rf /' to execute dangerous commands.

To mitigate this, you should pass command arguments as a list and avoid shell=True. This treats user input as a single, safe argument rather than executable code.

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", f"--metadata=title={report_title}"]
result = subprocess.run(command, capture_output=True, text=True)


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Using shell=True with subprocess.run is a security risk, as it can allow for shell injection vulnerabilities. When shell=True, the command is executed through the shell, which can interpret special characters and execute arbitrary commands.

When possible, it is best to avoid shell=True and instead pass the command as a list of arguments. This is safer because it does not involve the shell's parsing of the command string.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 The subprocess.run function is called with shell=True, which can be dangerous if the command is constructed from external input. In this case, report_title is passed directly into the command string, making it vulnerable to command injection.

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

Copy link
Copy Markdown

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 un-escaped user input in the subprocess.run function creates a command injection vulnerability. A malicious user could provide a title that includes shell commands, which would then be executed on the system.

To fix this, you should pass the command as a list of arguments and avoid using shell=True. This ensures that user input is treated as a single argument and not interpreted by the shell. Using check=True is also a good practice to automatically raise an exception if the command returns a non-zero exit code.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Using shell=True with subprocess.run is a security risk as it can allow for shell injection vulnerabilities. It's recommended to avoid it and instead pass the command as a list of arguments.

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

if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)
Comment on lines +3 to +15
Copy link
Copy Markdown

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 parameter is passed directly into a shell command that is executed with shell=True. A malicious user could provide a specially crafted title to execute arbitrary commands on the system.

Suggested change
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}'"
result = subprocess.run(command, shell=True, capture_output=True, text=True)
if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)
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, check=False)
if result.returncode == 0:
print("Report generated successfully.")
else:
print("Error:", result.stderr)


user_title = input("Enter report title: ")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The filename report_data.md is hardcoded. This makes the script less reusable. Consider making this a command-line argument, for example by using the argparse module.

generate_report(user_title, "report_data.md")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 The output filename report.pdf and the input filename report_data.md are hardcoded. It would be better to pass them as arguments to the function to make it more flexible.

Suggested change
generate_report(user_title, "report_data.md")
def generate_report(report_title, content_file, output_file):
"""Generates a PDF report with a user-supplied title."""
command = f"pandoc {content_file} -o {output_file} --metadata title='{report_title}'"
result = subprocess.run(command, shell=True, 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", "report.pdf")

Loading