Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove attacker-controlled error messages #456

Closed
deeplow opened this issue Jun 21, 2023 · 1 comment
Closed

Remove attacker-controlled error messages #456

deeplow opened this issue Jun 21, 2023 · 1 comment
Labels
security

Comments

@deeplow
Copy link
Contributor

deeplow commented Jun 21, 2023

When a document fails to convert it shows to the user an attacker-controlled error message.

We should reduce the risk of having the attacker showing the user some message that tells the user to bypass dangerzone and open the doc directly. Or at least make the user understand that the text is attacker-controlled.

We don't want to be in a situation like this:

datapath
@deeplow
Copy link
Contributor Author

deeplow commented Jun 21, 2023

One way we're thinking of tackling this is through the use of exit codes. Each conversion exit code maps to an issue in a particular step. Then the dangerzone application will show a text association with that error. If further debugging information is needed it can be obtained by running in the CLI with a special parameter (kind of like Qubes' --pass-io).

deeplow added a commit that referenced this issue Aug 30, 2023
@deeplow
Add server-side exception (passed via exit codes)
11ad6c2 
Creates exceptions in the server code to be shared with the client via an
identifying error code. These exceptions are then reconstructed in the
client.

This removes the need to have attacker controlled error messages passed
on to the client as prominently (perhaps only as extra verbose logging).

Refs #456 but does not solve it completely (containers code not covered)
deeplow added a commit that referenced this issue Aug 31, 2023
@deeplow
Remove attacker-controlled error messages
4c49d00 
Creates exceptions in the server code to be shared with the client via an
identifying exit code. These exceptions are then reconstructed in the
client.

Refes #456 but does not completely fix it. Unexpected exceptions and
progress descriptions are still passed in Containers.
deeplow added a commit that referenced this issue Aug 31, 2023
@deeplow
Remove attacker-controlled error messages
1530930 
Creates exceptions in the server code to be shared with the client via an
identifying exit code. These exceptions are then reconstructed in the
client.

Refes #456 but does not completely fix it. Unexpected exceptions and
progress descriptions are still passed in Containers.
deeplow added a commit that referenced this issue Aug 31, 2023
@deeplow
Remove attacker-controlled error messages
96d7308 
Creates exceptions in the server code to be shared with the client via an
identifying exit code. These exceptions are then reconstructed in the
client.

Refs #456 but does not completely fix it. Unexpected exceptions and
progress descriptions are still passed in Containers.
@deeplow deeplow mentioned this issue Aug 31, 2023
Merged
deeplow added a commit that referenced this issue Aug 31, 2023
@deeplow
Remove attacker-controlled error messages
57795d3 
Creates exceptions in the server code to be shared with the client via an
identifying exit code. These exceptions are then reconstructed in the
client.

Refs #456 but does not completely fix it. Unexpected exceptions and
progress descriptions are still passed in Containers.
deeplow added a commit that referenced this issue Aug 31, 2023
@deeplow
Remove attacker-controlled error messages
9871876 
Creates exceptions in the server code to be shared with the client via an
identifying exit code. These exceptions are then reconstructed in the
client.

Refs #456 but does not completely fix it. Unexpected exceptions and
progress descriptions are still passed in Containers.
deeplow added a commit that referenced this issue Sep 19, 2023
@deeplow
Remove attacker-controlled error messages
b4c3e07 
Creates exceptions in the server code to be shared with the client via an
identifying exit code. These exceptions are then reconstructed in the
client.

Refs #456 but does not completely fix it. Unexpected exceptions and
progress descriptions are still passed in Containers.
deeplow added a commit that referenced this issue Jan 9, 2024
@deeplow
Remove untrusted progress parsing (stderr instead)
a5e004d 
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.

The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.

Fixes #456
deeplow added a commit that referenced this issue Jan 10, 2024
@deeplow
Remove untrusted progress parsing (stderr instead)
abd3bab 
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.

The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.

Fixes #456
deeplow added a commit that referenced this issue Jan 10, 2024
@deeplow
Remove untrusted progress parsing (stderr instead)
438ddf1 
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.

The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.

Fixes #456
apyrgio pushed a commit that referenced this issue Jan 31, 2024
@deeplow @apyrgio
Remove untrusted progress parsing (stderr instead)
efaf01c 
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.

The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.

Fixes #456
deeplow added a commit that referenced this issue Feb 5, 2024
@deeplow
Remove untrusted progress parsing (stderr instead)
eb46049 
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.

The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.

Fixes #456
apyrgio pushed a commit that referenced this issue Feb 6, 2024
@deeplow @apyrgio
Remove untrusted progress parsing (stderr instead)
58d0d81 
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.

The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.

Fixes #456
deeplow added a commit that referenced this issue Feb 6, 2024
@deeplow
Remove untrusted progress parsing (stderr instead)
2e7fd30 
Now that only the second container can send JSON-encoded progress
information, we can the untrusted JSON parsing. The parse_progress was
also renamed to `parse_progress_trusted` to ensure future developers
don't mistake this as a safe method.

The old methods for sending untrusted JSON were repurposed to send the
progress instead to stderr for troubleshooting in development mode.

Fixes #456
@deeplow deeplow closed this as completed in 550786a Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
security
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@deeplow