π‘οΈ Sentinel: [CRITICAL] Fix pdflatex and pandoc Remote Code Execution (RCE) via shell escape#319
Conversation
β¦and DoS vulnerabilities Add `-no-shell-escape` flags and 30-second timeouts with explicit process cleanup to all PDF compilation subprocess calls to prevent arbitrary code execution via `\write18` and infinite loop DoS attacks. 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 GuideHardened LaTeX/PDF compilation by disabling shell escapes, adding timeouts, and ensuring robust subprocess cleanup for both pdflatex and pandoc paths, plus documenting the vulnerability in the Sentinel security log. Sequence diagram for hardened LaTeX PDF compilation with timeout and sandboxingsequenceDiagram
participant Caller as CoverLetterGenerator
participant Converter as _compile_pdf/_compile_pdflatex
participant Subprocess as subprocess
Caller->>Converter: _compile_pdf / _compile_pdflatex(tex_path, output_path)
Converter->>Subprocess: Popen([pdflatex|-pandoc, -no-shell-escape], cwd)
activate Subprocess
alt within_30_seconds
Converter->>Subprocess: process.communicate(timeout=30)
Subprocess-->>Converter: stdout, stderr
deactivate Subprocess
Converter->>Converter: check process.returncode or output_path.exists()
Converter-->>Caller: True/False
else timeout_Expired
Converter->>Subprocess: process.kill()
Subprocess-->>Converter: communicate()
Converter-->>Caller: False
end
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 30-second timeout is hard-coded in multiple places; consider centralizing this value (and possibly making it configurable) to avoid duplication and ease future tuning.
- The subprocess invocation and timeout/cleanup pattern is repeated for both pdflatex and pandoc; factoring this into a small helper function would reduce duplication and potential for divergence.
- On timeout you currently discard stderr/stdout after killing the process; consider logging or surfacing at least a summarized error to help diagnose recurring hangs or misconfigurations in LaTeX/pandoc.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The 30-second timeout is hard-coded in multiple places; consider centralizing this value (and possibly making it configurable) to avoid duplication and ease future tuning.
- The subprocess invocation and timeout/cleanup pattern is repeated for both pdflatex and pandoc; factoring this into a small helper function would reduce duplication and potential for divergence.
- On timeout you currently discard stderr/stdout after killing the process; consider logging or surfacing at least a summarized error to help diagnose recurring hangs or misconfigurations in LaTeX/pandoc.Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
π¨ Severity: CRITICAL
π‘ Vulnerability: PDF compilation commands (
pdflatexandpandocfallbacks) executed untrusted.textemplates without sandboxing (-no-shell-escape) and without timeouts, exposing the system to Remote Code Execution (RCE) via\write18and Denial of Service (DoS) via infinite compilation loops. Even fallback processes likepandocact as vectors for LaTeX-based exploits if they trigger a LaTeX engine under the hood.π― Impact: Malicious actors could use hallucinatory AI inputs or malicious prompt injections to generate LaTeX payloads that execute arbitrary shell commands on the host machine or intentionally hang the compilation process indefinitely.
π§ Fix:
-no-shell-escapeflag to allpdflatexsubprocess calls incli/pdf/converter.pyandcli/generators/cover_letter_generator.py.--pdf-engine-opt=-no-shell-escapeflag to allpandocfallback calls.process.kill()and a subsequent.communicate()) inexcept subprocess.TimeoutExpiredblocks to prevent zombie processes and double-free issues.β Verification: Verified by successfully running the full test suite (
python -m pytest) which showed 681 tests passing. Reviewed modified files to ensure correct flag placement and robust cleanup logic.PR created automatically by Jules for task 17456038898672213850 started by @anchapin
Summary by Sourcery
Harden LaTeX-based PDF generation to prevent remote code execution and hangs in compilation subprocesses.
Bug Fixes:
Enhancements:
Documentation: