Sanitize sandbox output #491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apyrgio
force-pushed
the
2023-07-sanitize-progress-update
branch
from
3e5e280 to
2d3457d
Compare
deeplow
approved these changes
Jul 31, 2023
deeplow
reviewed
Aug 1, 2023
The isort tool is not compatible with Black by default. This leads to a tug of war between these tools, when we run `make lint-apply` -> `make lint`. Fix this by forcing isort to be compatible with Black.
Do not forget to reset the red text once we print an error string to the terminal
Run our GUI tests on separate processes, because the combination of Ubuntu Focal, Qt5, PySide6, and pytest-qt somehow leads to segfaults, probably due to stale global state. Closes #493
Make the `error_label` widget always render messages as plain text, instead of auto discovering if the text is rich. We need this because the error message may contain input from the sandbox, which we consider untrusted.
Add `replace_control_chars()` function in `util.py`, which can be used to sanitize strings from ANSI escape sequences or weird Unicode symbols.
Sanitize filenames in various places in the code, before we write them to the user's terminal. Filenames, especially in Linux, can contain virtually any character except for '\0' and '/', so it's important to sanitize them.
Update the common `print_progress()` method in the base `IsolationProvider` class, with two extra features: 1. Always sanitize the provided text argument. 2. Mark the sanitized text argument as untrusted. This is default behavior from now on, since this function is commonly used to parse progress reports from the conversion sandbox.
Improve the `parse_progress()` method of the container isolation provider in the following ways: 1. Make sure that the fields of the progress report have the expected type. 2. In case of a JSON parsing error, sanitize the invalid string so that it doesn't contain escape sequences, or the user considers it as trusted.
Mark the messages that Dangerzone creates once a conversion step finishes as trusted, since they do not contain any string not controlled by us.
apyrgio
force-pushed
the
2023-07-sanitize-progress-update
branch
from
09f866f to
6c374d8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sanitize the output that the container/disposable VM sends back to the user, before displaying it on screen/terminal. Also tackle some issues that are closely related to the sanitization logic, and add some tests.