diff --git a/CHANGELOG.md b/CHANGELOG.md index f615ac4..23049b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ since 0.4.1, and this project adheres to [Semantic Versioning](https://semver.or - Continuously scan our Python dependencies and container image for vulnerabilities ([issue #222](https://github.com/freedomofpress/dangerzone/issues/222)) +- Sanitize potentially unsafe characters from strings that are shown in a user's + terminal. ## Dangerzone 0.4.1 diff --git a/dangerzone/isolation_provider/base.py b/dangerzone/isolation_provider/base.py index aa02c54..38f02c7 100644 --- a/dangerzone/isolation_provider/base.py +++ b/dangerzone/isolation_provider/base.py @@ -6,6 +6,7 @@ from typing import Callable, Optional from colorama import Fore, Style from ..document import Document +from ..util import replace_control_chars log = logging.getLogger(__name__) @@ -49,7 +50,7 @@ class IsolationProvider(ABC): ) -> bool: pass - def print_progress( + def _print_progress( self, document: Document, error: bool, text: str, percentage: float ) -> None: s = Style.BRIGHT + Fore.YELLOW + f"[doc {document.id}] " @@ -64,6 +65,19 @@ class IsolationProvider(ABC): if self.progress_callback: self.progress_callback(error, text, percentage) + def print_progress_trusted( + self, document: Document, error: bool, text: str, percentage: float + ) -> None: + return self._print_progress(document, error, text, percentage) + + def print_progress( + self, document: Document, error: bool, untrusted_text: str, percentage: float + ) -> None: + text = replace_control_chars(untrusted_text) + return self.print_progress_trusted( + document, error, "UNTRUSTED> " + text, percentage + ) + @abstractmethod def get_max_parallel_conversions(self) -> int: pass diff --git a/tests/isolation_provider/test_base.py b/tests/isolation_provider/test_base.py new file mode 100644 index 0000000..c87acc7 --- /dev/null +++ b/tests/isolation_provider/test_base.py @@ -0,0 +1,56 @@ +import pytest +from colorama import Style +from pytest_mock import MockerFixture + +from dangerzone.document import Document +from dangerzone.isolation_provider import base, container, qubes + +from .. import sanitized_text, uncommon_text + + +@pytest.mark.parametrize( + "provider", + [ + container.Container(enable_timeouts=False), + qubes.Qubes(), + ], + ids=["Container", "Qubes"], +) +def test_print_progress( + provider: base.IsolationProvider, + uncommon_text: str, + sanitized_text: str, + mocker: MockerFixture, +) -> None: + """Test that the print_progress() method of our isolation providers sanitizes text. + + Iterate our isolation providers and make sure that their print_progress() methods + sanitizes the provided text, before passing it to the logging functions and other + callbacks. + """ + d = Document() + provider.progress_callback = mocker.MagicMock() + log_info_spy = mocker.spy(base.log, "info") + log_error_spy = mocker.spy(base.log, "error") + _print_progress_spy = mocker.spy(provider, "_print_progress") + + for error, untrusted_text, sanitized_text in [ + (True, "normal text", "UNTRUSTED> normal text"), + (False, "normal text", "UNTRUSTED> normal text"), + (True, uncommon_text, "UNTRUSTED> " + sanitized_text), + (False, uncommon_text, "UNTRUSTED> " + sanitized_text), + ]: + log_info_spy.reset_mock() + log_error_spy.reset_mock() + + provider.print_progress(d, error, untrusted_text, 0) + provider.progress_callback.assert_called_with(error, sanitized_text, 0) # type: ignore [union-attr] + _print_progress_spy.assert_called_with(d, error, sanitized_text, 0) + if error: + assert log_error_spy.call_args[0][0].endswith( + sanitized_text + Style.RESET_ALL + ) + log_info_spy.assert_not_called() + else: + assert log_info_spy.call_args[0][0].endswith(sanitized_text) + log_error_spy.assert_not_called()