🛡️ Sentinel: [CRITICAL] Fix LaTeX RCE and DoS vulnerabilities#328
🛡️ Sentinel: [CRITICAL] Fix LaTeX RCE and DoS vulnerabilities#328anchapin wants to merge 2 commits into
Conversation
Severity: CRITICAL Vulnerability: Remote Code Execution (RCE) via LaTeX shell escape and Denial of Service (DoS) via infinite compilation loops in pdflatex and pandoc subprocesses. Impact: An attacker or AI hallucination could inject malicious LaTeX commands (like \write18) to execute arbitrary shell commands on the server, or provide malformed LaTeX to hang the compilation process indefinitely, starving resources. Fix: Added `-no-shell-escape` flag to both pdflatex and pandoc (via --pdf-engine-opt) to strictly disable shell command execution. Implemented 30-second timeouts on all subprocess.communicate() calls, properly killing and cleaning up processes if they hang. Verification: Ran the unit test suite natively and verified via `git diff` that the correct arguments and exception handling blocks are correctly in place. 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 LaTeX PDF generation by disabling shell escape for pdflatex/pandoc and adding time-bounded subprocess handling to prevent RCE and DoS during PDF compilation in both the cover letter generator and generic PDF converter, plus documenting the issue in Sentinel notes. Sequence diagram for hardened LaTeX PDF compilation with timeout and no shell escapesequenceDiagram
participant CoverLetterGenerator
participant pdflatex
participant pandoc
CoverLetterGenerator->>pdflatex: Popen(["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name])
alt pdflatex completes within 30s
pdflatex-->>CoverLetterGenerator: communicate(timeout=30)
alt pdflatex success or output_path.exists()
CoverLetterGenerator->>CoverLetterGenerator: pdf_created = True
else pdflatex failure
CoverLetterGenerator->>CoverLetterGenerator: pdf_created = False
end
else pdflatex hangs
CoverLetterGenerator->>pdflatex: kill()
pdflatex-->>CoverLetterGenerator: communicate()
CoverLetterGenerator->>CoverLetterGenerator: pdf_created = False
CoverLetterGenerator->>pandoc: Popen(["pandoc", tex_path, "-o", output_path, "--pdf-engine=xelatex", "--pdf-engine-opt=-no-shell-escape"])
alt pandoc completes within 30s
pandoc-->>CoverLetterGenerator: communicate(timeout=30)
alt pandoc success or output_path.exists()
CoverLetterGenerator->>CoverLetterGenerator: pdf_created = True
else pandoc failure
CoverLetterGenerator->>CoverLetterGenerator: pdf_created = False
end
else pandoc hangs
CoverLetterGenerator->>pandoc: kill()
pandoc-->>CoverLetterGenerator: communicate()
CoverLetterGenerator->>CoverLetterGenerator: pdf_created = False
end
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 timeout handling is inconsistent between
cover_letter_generator._compile_pdfandpdf.converter(one setspdf_created = Falseand continues, the other returns early); consider unifying the behavior so that a timed-out compilation is always treated the same way and does not fall through to the success checks. - In
_compile_pdf, after aTimeoutExpiredyou still run theif process.returncode == 0 or output_path.exists()block; if a timeout should be a hard failure, it would be safer to return immediately or guard that success check so a partially written or stale PDF doesn’t cause the compile to be reported as successful.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The timeout handling is inconsistent between `cover_letter_generator._compile_pdf` and `pdf.converter` (one sets `pdf_created = False` and continues, the other returns early); consider unifying the behavior so that a timed-out compilation is always treated the same way and does not fall through to the success checks.
- In `_compile_pdf`, after a `TimeoutExpired` you still run the `if process.returncode == 0 or output_path.exists()` block; if a timeout should be a hard failure, it would be safer to return immediately or guard that success check so a partially written or stale PDF doesn’t cause the compile to be reported as successful.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 LaTeX shell escape and Denial of Service (DoS) via infinite compilation loops in pdflatex and pandoc subprocesses. Impact: An attacker or AI hallucination could inject malicious LaTeX commands (like \write18) to execute arbitrary shell commands on the server, or provide malformed LaTeX to hang the compilation process indefinitely, starving resources. Fix: Added `-no-shell-escape` flag to both pdflatex and pandoc (via --pdf-engine-opt) to strictly disable shell command execution. Implemented 30-second timeouts on all subprocess.communicate() calls, properly killing and cleaning up processes if they hang. Verification: Ran the unit test suite natively and verified via `git diff` that the correct arguments and exception handling blocks are correctly in place. Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
🛡️ Sentinel: [CRITICAL] Fix LaTeX RCE and DoS vulnerabilities
🚨 Severity: CRITICAL
💡 Vulnerability: Remote Code Execution (RCE) via LaTeX shell escape and Denial of Service (DoS) via infinite compilation loops in pdflatex and pandoc subprocesses.
🎯 Impact: An attacker or AI hallucination could inject malicious LaTeX commands (like \write18) to execute arbitrary shell commands on the server, or provide malformed LaTeX to hang the compilation process indefinitely, starving resources.
🔧 Fix: Added
-no-shell-escapeflag to both pdflatex and pandoc (via --pdf-engine-opt) to strictly disable shell command execution. Implemented 30-second timeouts on all subprocess.communicate() calls, properly killing and cleaning up processes if they hang.✅ Verification: Ran the unit test suite natively and verified via
git diffthat the correct arguments and exception handling blocks are correctly in place. Also documented learning in.jules/sentinel.md.PR created automatically by Jules for task 16815568420048628348 started by @anchapin
Summary by Sourcery
Harden LaTeX-based PDF generation against remote code execution and denial-of-service conditions.
Bug Fixes:
Documentation: