diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 3a1d237..ed4adbe 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -7,3 +7,8 @@ **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. + +## 2025-05-07 - [Critical] RCE Vulnerability in PDF Compilation +**Vulnerability:** The application used `subprocess.Popen` to compile LaTeX files using `pdflatex` and `pandoc` without the `-no-shell-escape` flag. While interaction mode was non-stop, omitting this flag allows malicious `.tex` content (e.g., injected via unsanitized user input or AI hallucinations) to execute arbitrary shell commands via the `\write18` feature or similar LaTeX exploits. Furthermore, the compilation lacked process timeouts, allowing for potential Denial of Service (DoS) attacks via infinite compilation loops. +**Learning:** External compilation tools that interpret complex document formats (like LaTeX or Markdown) often have built-in shell execution capabilities for advanced features. These must be explicitly disabled when compiling untrusted input. Additionally, blocking processes must always have bounded execution times to prevent resource exhaustion. +**Prevention:** Always append `-no-shell-escape` to `pdflatex` commands (and equivalent options like `--pdf-engine-opt=-no-shell-escape` for `pandoc`). Always implement explicit timeouts (e.g., `process.communicate(timeout=30)`) combined with cleanup logic (`process.kill()`) when calling external blocking executables. diff --git a/cli/generators/cover_letter_generator.py b/cli/generators/cover_letter_generator.py index aaf0b61..f5b0840 100644 --- a/cli/generators/cover_letter_generator.py +++ b/cli/generators/cover_letter_generator.py @@ -771,15 +771,21 @@ 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() + raise RuntimeError("PDF compilation timed out") + if process.returncode == 0 or output_path.exists(): pdf_created = True - except (subprocess.CalledProcessError, FileNotFoundError): + except (subprocess.CalledProcessError, FileNotFoundError, RuntimeError): # Check if PDF was created anyway if output_path.exists(): pdf_created = True @@ -787,14 +793,27 @@ 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() + raise RuntimeError("PDF compilation timed out") + if process.returncode == 0 or output_path.exists(): pdf_created = True - except (subprocess.CalledProcessError, FileNotFoundError): + except (subprocess.CalledProcessError, FileNotFoundError, RuntimeError): pass if not pdf_created or not output_path.exists(): diff --git a/cli/pdf/converter.py b/cli/pdf/converter.py index 0b0a200..5499f72 100644 --- a/cli/pdf/converter.py +++ b/cli/pdf/converter.py @@ -86,16 +86,21 @@ 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() + raise RuntimeError("PDF compilation timed out") if process.returncode == 0 or output_path.exists(): return True - except (subprocess.CalledProcessError, FileNotFoundError): + except (subprocess.CalledProcessError, FileNotFoundError, RuntimeError): # Check if PDF was created anyway (pdflatex returns non-zero for warnings) if output_path.exists(): return True @@ -121,16 +126,28 @@ 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() + raise RuntimeError("PDF compilation timed out") if process.returncode == 0 or output_path.exists(): return True - except (subprocess.CalledProcessError, FileNotFoundError): + except (subprocess.CalledProcessError, FileNotFoundError, RuntimeError): pass return False