π‘οΈ Sentinel: [CRITICAL] Fix Remote Code Execution in PDF Compilers#318
π‘οΈ Sentinel: [CRITICAL] Fix Remote Code Execution in PDF Compilers#318anchapin wants to merge 1 commit into
Conversation
Addresses a critical RCE vulnerability where `pdflatex` and `pandoc` were invoked without strictly restricting shell execution or applying timeout bounds. A maliciously crafted input could inject commands using `\write18` to execute arbitrary shell commands on the host machine. The fix enforces the `-no-shell-escape` flag for `pdflatex` and `--pdf-engine-opt=-no-shell-escape` for `pandoc`, and implements a 30-second subprocess timeout with proper process cleanup (`process.kill()`) to prevent infinite compilation loops (DoS). Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
|
π Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a π emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's GuideHarden PDF compilation by disabling LaTeX shell execution for both primary and fallback compilers and by enforcing a 30-second timeout with proper subprocess cleanup to mitigate RCE and DoS risks. Sequence diagram for secured PDF compilation with timeout and no shell escapesequenceDiagram
participant Caller as CoverLetterGenerator
participant Converter as _compile_pdflatex
participant Popen as subprocess_Popen
Caller->>Converter: _compile_pdflatex(tex_path, output_path, working_dir)
Converter->>Popen: Popen(["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name])
activate Popen
Converter->>Popen: communicate(timeout=30)
alt compilation completes in time
Popen-->>Converter: stdout, stderr
Converter->>Converter: check returncode or output_path.exists()
Converter-->>Caller: True or False
else subprocess.TimeoutExpired
Converter->>Popen: kill()
Popen-->>Converter: communicate()
Converter-->>Caller: RuntimeError("PDF compilation timed out")
end
deactivate Popen
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The subprocess timeout/kill/RuntimeError pattern is duplicated across the pdflatex and pandoc paths; consider extracting a small helper (e.g.,
run_with_timeout(cmd, cwd=None, timeout=30)) to reduce repetition and keep the behavior consistent in one place. - The 30-second timeout is currently hard-coded in multiple places; you may want to centralize this as a configuration constant or parameter so it can be tuned per environment without code changes.
- When the PDF compilation times out or fails, you currently discard
stdout/stderr; consider including relevant portions ofstderrin the raised exception or log to aid debugging operational issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The subprocess timeout/kill/RuntimeError pattern is duplicated across the pdflatex and pandoc paths; consider extracting a small helper (e.g., `run_with_timeout(cmd, cwd=None, timeout=30)`) to reduce repetition and keep the behavior consistent in one place.
- The 30-second timeout is currently hard-coded in multiple places; you may want to centralize this as a configuration constant or parameter so it can be tuned per environment without code changes.
- When the PDF compilation times out or fails, you currently discard `stdout`/`stderr`; consider including relevant portions of `stderr` in the raised exception or log to aid debugging operational issues.Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
π¨ Severity: CRITICAL
π‘ Vulnerability: Remote Code Execution (RCE) via
\write18in LaTeX compilation, and Denial of Service (DoS) via missing timeouts.π― Impact: Maliciously crafted inputs or AI hallucinations could execute arbitrary shell commands on the host system or cause infinite processing loops.
π§ Fix:
-no-shell-escapeflag topdflatexcalls.--pdf-engine-opt=-no-shell-escapetopandocfallback calls.process.communicate(timeout=30)with appropriate process termination on expiration.β Verification:
pytest tests/test_pdf_security.py)PR created automatically by Jules for task 17041182512615523487 started by @anchapin
Summary by Sourcery
Harden PDF compilation against LaTeX-based remote code execution and unbounded execution during PDF generation.
Bug Fixes:
Documentation: