From 9f1abe28360925296e6006b4b003a15f63900bde Mon Sep 17 00:00:00 2001 From: deeplow Date: Fri, 23 Jun 2023 07:32:29 +0100 Subject: [PATCH] Replace non-printable ascii in conversion log Certain characters may be abused. Particularly ANSI escape codes. Solution inspired by Qubes OS's hardening of ther RPC mechanism [1]: > Terminal control characters are a security issue, which in worst case > amount to arbitrary command execution. In the simplest case this > requires two often found codes: terminal title setting (which puts > arbitrary string in the window title) and title repo reporting (which > puts that string on the shell's standard input. [sic] > > -- qvm-run.rst [2] [1]: https://github.com/QubesOS/qubes-core-admin-linux/commit/e005836286ed4d5615c34608a088a30d9aa7a556 [2]: https://github.com/QubesOS/qubes-core-admin-client/blob/c70da44702109b45d960c7de170ab8c2820b7167/doc/manpages/qvm-run.rst?plain=1#L126 --- dangerzone/isolation_provider/container.py | 14 ++++++++++---- dangerzone/isolation_provider/qubes.py | 6 ++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/dangerzone/isolation_provider/container.py b/dangerzone/isolation_provider/container.py index abc0100..6f44316 100644 --- a/dangerzone/isolation_provider/container.py +++ b/dangerzone/isolation_provider/container.py @@ -12,7 +12,12 @@ import tempfile from typing import Any, Callable, List, Optional, Tuple from ..document import Document -from ..util import get_resource_path, get_subprocess_startupinfo, get_tmp_dir +from ..util import ( + get_resource_path, + get_subprocess_startupinfo, + get_tmp_dir, + replace_control_chars, +) from .base import MAX_CONVERSION_LOG_CHARS, IsolationProvider # Define startupinfo for subprocesses @@ -288,9 +293,10 @@ class Container(IsolationProvider): if getattr(sys, "dangerzone_dev", False): log_path = pixel_dir / "captured_output.txt" with open(log_path, "r", encoding="ascii", errors="replace") as f: - log.info( - f"Conversion output (doc to pixels):\n{f.read(MAX_CONVERSION_LOG_CHARS)}" - ) + untrusted_log = f.read(MAX_CONVERSION_LOG_CHARS) + log.info( + f"Conversion output (doc to pixels):\n{replace_control_chars(untrusted_log)}" + ) if ret != 0: log.error("documents-to-pixels failed") diff --git a/dangerzone/isolation_provider/qubes.py b/dangerzone/isolation_provider/qubes.py index 0269d92..167858c 100644 --- a/dangerzone/isolation_provider/qubes.py +++ b/dangerzone/isolation_provider/qubes.py @@ -143,8 +143,10 @@ class Qubes(IsolationProvider): self.print_progress_trusted(document, False, text, percentage) if getattr(sys, "dangerzone_dev", False): - text = f"Conversion output (doc to pixels):\n{read_debug_text(p)}" - log.info(text) + untrusted_log = read_debug_text(p) + log.info( + f"Conversion output (doc to pixels):\n{replace_control_chars(untrusted_log)}" + ) # FIXME pass OCR stuff properly (see #455) old_environ = dict(os.environ)