🛡️ Sentinel: [CRITICAL] Fix pdflatex RCE and DoS vulnerabilities#341
🛡️ Sentinel: [CRITICAL] Fix pdflatex RCE and DoS vulnerabilities#341anchapin wants to merge 1 commit into
Conversation
Enforce -no-shell-escape flag and explicit 30-second timeouts on all pdflatex and pandoc subprocess executions in cli/pdf/converter.py and cli/generators/cover_letter_generator.py to prevent LaTeX-based Remote Code Execution and infinite loop Denial of Service vulnerabilities via untrusted input. Also correctly handles TimeoutExpired to prevent zombie processes and prevents false-positive successful returns on timeouts. 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 generation by disabling shell escape and adding timeouts around pdflatex/pandoc subprocesses, plus documenting the security issue in Sentinel notes. Sequence diagram for hardened pdflatex compilation with timeout and no-shell-escapesequenceDiagram
participant Caller as PdfConverter
participant Converter as _compile_pdflatex
participant Subproc as subprocess.Popen
participant Proc as process
Caller->>Converter: _compile_pdflatex(tex_path, working_dir, output_path)
Converter->>Subproc: Popen(["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name])
Note right of Subproc: returns process
Subproc-->>Converter: process
alt process completes within 30s
Converter->>Proc: communicate(timeout=30)
Proc-->>Converter: stdout, stderr
Converter->>Caller: return True or False based on
Note right of Converter: True if returncode==0 or output_path.exists()
else subprocess.TimeoutExpired
Converter->>Proc: kill()
Converter->>Proc: communicate()
Proc-->>Converter: stdout, stderr
Converter->>Caller: return 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 subprocess timeout/kill logic is duplicated across pdflatex and pandoc calls; consider extracting a small helper (e.g.,
run_with_timeout(cmd, cwd, timeout=30)) to centralize behavior and make future changes less error-prone. - The hardcoded
timeout=30value is repeated in multiple places; promoting this to a shared constant (or configuration option) would make it easier to adjust globally if needed and keep the timeout policy consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The subprocess timeout/kill logic is duplicated across pdflatex and pandoc calls; consider extracting a small helper (e.g., `run_with_timeout(cmd, cwd, timeout=30)`) to centralize behavior and make future changes less error-prone.
- The hardcoded `timeout=30` value is repeated in multiple places; promoting this to a shared constant (or configuration option) would make it easier to adjust globally if needed and keep the timeout policy consistent.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 via
pdflatexandpandocincli/pdf/converter.pyandcli/generators/cover_letter_generator.pyomitted the-no-shell-escapeflag and did not enforce an execution timeout.Impact: This allowed malicious untrusted input to execute arbitrary shell commands via LaTeX (
\write18), leading to Remote Code Execution (RCE), as well as infinite compilation loops leading to Denial of Service (DoS).Fix: Explicitly added
-no-shell-escape(or--pdf-engine-opt=-no-shell-escapefor pandoc) to all execution commands. Additionally addedtimeout=30toprocess.communicate()to prevent hangs, and correctly caughtsubprocess.TimeoutExpiredto kill the zombie processes and safely skip completion logic.Verification: Verified via manual source checking via sed and ran full test suite (
python -m pytest tests/) to ensure no regressions. All 681 tests passed successfully.PR created automatically by Jules for task 8184241153704705132 started by @anchapin
Summary by Sourcery
Harden PDF generation against RCE and DoS via external LaTeX tools.
Bug Fixes:
Documentation: