Skip to content

🛡️ Sentinel: [High] Fix RCE and DoS in PDF compilation #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anchapin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🛡️ Sentinel: [High] Fix RCE and DoS in PDF compilation #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@
**Vulnerability:** The `CoverLetterGenerator` used a standard Jinja2 environment (intended for HTML/XML or plain text) to render LaTeX templates. This allowed malicious user input (or AI hallucinations) containing LaTeX control characters (e.g., `\input{...}`) to be injected directly into the LaTeX source, leading to potential Local File Inclusion (LFI) or other exploits.
**Learning:** Jinja2's default `autoescape` is context-aware based on file extensions, but usually only for HTML/XML. It does NOT automatically escape LaTeX special characters. Relying on manual filters (like `| latex_escape`) in templates is error-prone and brittle, as developers might forget to apply them to every variable.
**Prevention:** Always use a dedicated Jinja2 environment for LaTeX generation that enforces auto-escaping via a `finalize` hook (e.g., `tex_env.finalize = latex_escape`). This ensures *all* variable output is sanitized by default, providing defense-in-depth even if the template author forgets explicit filters.
## 2026-04-28 - [High] RCE and DoS in Subprocess PDF Compilation
**Vulnerability:** The `PDFConverter` and `CoverLetterGenerator` modules used `subprocess.Popen` to execute `pdflatex` and `pandoc` without the `-no-shell-escape` flag, allowing arbitrary command execution if malicious LaTeX was compiled. Additionally, the `communicate()` calls lacked timeouts, allowing a malicious payload to cause a Denial of Service (DoS) via an infinite compilation loop.
**Learning:** Security fixes applied to a core module (like `TemplateGenerator`) often leave identical vulnerabilities in duplicated or extracted code (like `PDFConverter` and `CoverLetterGenerator`). All instances of external command execution must be hardened uniformly.
**Prevention:** Always search the entire codebase for similar usages of vulnerable patterns (e.g., `grep -rn 'subprocess.Popen' .`) when fixing an issue to ensure comprehensive coverage. Always enforce timeouts and explicit process cleanup when using `subprocess`.
23 changes: 19 additions & 4 deletions cli/generators/cover_letter_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,12 +771,16 @@ def _compile_pdf(self, output_path: Path, tex_content: str) -> bool:
try:
# Use Popen with explicit cleanup to avoid double-free issues
process = subprocess.Popen(
["pdflatex", "-interaction=nonstopmode", tex_path.name],
["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=tex_path.parent,
)
stdout, stderr = process.communicate()
try:
stdout, stderr = process.communicate(timeout=30)
except subprocess.TimeoutExpired:
process.kill()
stdout, stderr = process.communicate()
if process.returncode == 0 or output_path.exists():
pdf_created = True
except (subprocess.CalledProcessError, FileNotFoundError):
Expand All @@ -787,11 +791,22 @@ def _compile_pdf(self, output_path: Path, tex_content: str) -> bool:
# Fallback to pandoc
try:
process = subprocess.Popen(
["pandoc", str(tex_path), "-o", str(output_path), "--pdf-engine=xelatex"],
[
"pandoc",
str(tex_path),
"-o",
str(output_path),
"--pdf-engine=xelatex",
"--pdf-engine-opt=-no-shell-escape",
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
stdout, stderr = process.communicate()
try:
stdout, stderr = process.communicate(timeout=30)
except subprocess.TimeoutExpired:
process.kill()
stdout, stderr = process.communicate()
if process.returncode == 0 or output_path.exists():
pdf_created = True
except (subprocess.CalledProcessError, FileNotFoundError):
Expand Down
23 changes: 19 additions & 4 deletions cli/pdf/converter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,16 @@ def _compile_pdflatex(
"""
try:
process = subprocess.Popen(
["pdflatex", "-interaction=nonstopmode", tex_path.name],
["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=working_dir,
)
stdout, stderr = process.communicate()
try:
stdout, stderr = process.communicate(timeout=30)
except subprocess.TimeoutExpired:
process.kill()
stdout, stderr = process.communicate()

if process.returncode == 0 or output_path.exists():
return True
Expand Down Expand Up @@ -121,12 +125,23 @@ def _compile_pandoc(
"""
try:
process = subprocess.Popen(
["pandoc", str(tex_path), "-o", str(output_path), "--pdf-engine=xelatex"],
[
"pandoc",
str(tex_path),
"-o",
str(output_path),
"--pdf-engine=xelatex",
"--pdf-engine-opt=-no-shell-escape",
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=working_dir,
)
stdout, stderr = process.communicate()
try:
stdout, stderr = process.communicate(timeout=30)
except subprocess.TimeoutExpired:
process.kill()
stdout, stderr = process.communicate()

if process.returncode == 0 or output_path.exists():
return True
Expand Down
73 changes: 73 additions & 0 deletions tests/test_pdf_security.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from unittest.mock import MagicMock, patch

from cli.generators.template import TemplateGenerator
from cli.generators.cover_letter_generator import CoverLetterGenerator
from cli.pdf.converter import PDFConverter


class TestPDFSecurity(unittest.TestCase):
Expand Down Expand Up @@ -55,6 +57,77 @@ def test_pdflatex_arguments(self, mock_popen):
self.assertIn("-interaction=nonstopmode", command)
self.assertIn("pdflatex", command)

@patch("cli.pdf.converter.subprocess.Popen")
def test_converter_pdflatex_timeout_and_args(self, mock_popen):
process_mock = MagicMock()
process_mock.communicate.side_effect = [
subprocess.TimeoutExpired(cmd="pdflatex", timeout=30),
(b"", b""),
]
process_mock.returncode = 0
mock_popen.return_value = process_mock

converter = PDFConverter()

with patch.object(Path, "exists", return_value=True):
converter._compile_pdflatex(Path("output.tex"), Path("output.pdf"), Path("."))

process_mock.kill.assert_called_once()
process_mock.communicate.assert_any_call(timeout=30)

args, _ = mock_popen.call_args
command = args[0]
self.assertIn("-no-shell-escape", command)
self.assertIn("-interaction=nonstopmode", command)
self.assertIn("pdflatex", command)

@patch("cli.pdf.converter.subprocess.Popen")
def test_converter_pandoc_timeout_and_args(self, mock_popen):
process_mock = MagicMock()
process_mock.communicate.side_effect = [
subprocess.TimeoutExpired(cmd="pandoc", timeout=30),
(b"", b""),
]
process_mock.returncode = 0
mock_popen.return_value = process_mock

converter = PDFConverter()

with patch.object(Path, "exists", return_value=True):
converter._compile_pandoc(Path("output.tex"), Path("output.pdf"), Path("."))

process_mock.kill.assert_called_once()
process_mock.communicate.assert_any_call(timeout=30)

args, _ = mock_popen.call_args
command = args[0]
self.assertIn("--pdf-engine-opt=-no-shell-escape", command)
self.assertIn("pandoc", command)

@patch("subprocess.Popen")
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot Apr 28, 2026

Choose a reason for hiding this comment

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

issue (testing): Patch target for Popen in CoverLetterGenerator tests is incorrect and may call the real subprocess implementation

Because cover_letter_generator.py imports subprocess directly, the patch must target the symbol as used in that module (e.g. @patch("cli.generators.cover_letter_generator.subprocess.Popen")). Using @patch("subprocess.Popen") won’t intercept the call inside _compile_pdf and can trigger the real pdflatex during tests. Please update the patch target accordingly.

def test_cover_letter_pdflatex_timeout_and_args(self, mock_popen):
process_mock = MagicMock()
process_mock.communicate.side_effect = [
subprocess.TimeoutExpired(cmd="pdflatex", timeout=30),
(b"", b""),
]
process_mock.returncode = 0
mock_popen.return_value = process_mock

generator = CoverLetterGenerator()

with patch.object(Path, "exists", return_value=True):
generator._compile_pdf(Path("output.pdf"), "content")

process_mock.kill.assert_called_once()
process_mock.communicate.assert_any_call(timeout=30)

args, _ = mock_popen.call_args
command = args[0]
self.assertIn("-no-shell-escape", command)
self.assertIn("-interaction=nonstopmode", command)
self.assertIn("pdflatex", command)


if __name__ == "__main__":
unittest.main()
Loading