Skip to content

Commit

Permalink
Remove attacker-controlled error messages
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
deeplow committed Aug 31, 2023
1 parent c996140 commit 4c49d00
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 17 deletions.
26 changes: 13 additions & 13 deletions dangerzone/conversion/doc_to_pixels.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@
import re
import shutil
import sys
from typing import Dict, Optional
from typing import Dict, List, Optional

import magic

from .common import DangerzoneConverter, running_on_qubes
from .errors import *


class DocumentToPixels(DangerzoneConverter):
Expand Down Expand Up @@ -139,7 +140,7 @@ async def convert(self) -> None:

# Validate MIME type
if mime_type not in conversions:
raise ValueError("The document format is not supported")
raise DocFormatUnsupported()

# Temporary fix for the HWPX format
# Should be removed after new release of `file' (current release 5.44)
Expand Down Expand Up @@ -171,11 +172,9 @@ async def convert(self) -> None:
"arm64",
"aarch64",
):
raise ValueError(
"HWP / HWPX formats are not supported in ARM architectures"
)
raise DocFormatUnsupportedHWPArm()
if libreoffice_ext == "h2orestart.oxt" and running_on_qubes():
raise ValueError("HWP / HWPX formats are not supported in Qubes")
raise DocFormatUnsupportedHWPQubes()
if libreoffice_ext:
await self.install_libreoffice_ext(libreoffice_ext)
self.update_progress("Converting to PDF using LibreOffice")
Expand Down Expand Up @@ -204,7 +203,7 @@ async def convert(self) -> None:
#
# https://github.com/freedomofpress/dangerzone/issues/494
if not os.path.exists(pdf_filename):
raise ValueError("Conversion to PDF with LibreOffice failed")
raise LibreofficeFailure()
elif conversion["type"] == "convert":
self.update_progress("Converting to PDF using GraphicsMagick")
args = [
Expand All @@ -224,7 +223,7 @@ async def convert(self) -> None:
)
pdf_filename = "/tmp/input_file.pdf"
else:
raise ValueError(
raise InvalidGMConversion(
f"Invalid conversion type {conversion['type']} for MIME type {mime_type}"
)
self.percentage += 3
Expand All @@ -244,7 +243,7 @@ async def convert(self) -> None:
if search is not None:
num_pages: int = int(search.group(1))
else:
raise ValueError("Number of pages could not be extracted from PDF")
raise NoPageCountException()

# Get a more precise timeout, based on the number of pages
timeout = self.calculate_timeout(size, num_pages)
Expand Down Expand Up @@ -291,7 +290,7 @@ def pdftoppm_progress_callback(line: bytes) -> None:
# Read the header
header = f.readline().decode().strip()
if header != "P6":
raise ValueError("Invalid PPM header")
raise PDFtoPPMInvalidHeader()

# Save the width and height
dims = f.readline().decode().strip()
Expand All @@ -304,7 +303,7 @@ def pdftoppm_progress_callback(line: bytes) -> None:
maxval = int(f.readline().decode().strip())
# Check that the depth is 8
if maxval != 255:
raise ValueError("Invalid PPM depth")
raise PDFtoPPMInvalidDepth()

data = f.read()

Expand Down Expand Up @@ -378,10 +377,11 @@ async def main() -> int:
try:
await converter.convert()
error_code = 0 # Success!
except (RuntimeError, TimeoutError, ValueError) as e:
except ConversionException as e: # Expected Errors
error_code = e.error_code
except (RuntimeError, TimeoutError, ValueError) as e: # Unexpected Errors
converter.update_progress(str(e), error=True)
error_code = 1

if not running_on_qubes():
# Write debug information (containers version)
with open("/tmp/dangerzone/captured_output.txt", "wb") as container_log:
Expand Down
10 changes: 8 additions & 2 deletions dangerzone/conversion/doc_to_pixels_qubes_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from typing import Optional, TextIO

from .doc_to_pixels import DocumentToPixels
from .errors import ConversionException


def _read_bytes() -> bytes:
Expand Down Expand Up @@ -67,8 +68,13 @@ async def main() -> None:
with open("/tmp/input_file", "wb") as f:
f.write(data)

converter = DocumentToPixels()
await converter.convert()
try:
converter = DocumentToPixels()
await converter.convert()
except ConversionException as e:
sys.exit(e.error_code)
except (RuntimeError, TimeoutError, ValueError) as e:
sys.exit(1)

num_pages = len(list(out_dir.glob("*.rgb")))
await write_int(num_pages)
Expand Down
78 changes: 78 additions & 0 deletions dangerzone/conversion/errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
from typing import List, Optional


class ConversionException(Exception):
error_message = "Unspecificed error"
error_code = -1

def __init__(self, error_message: Optional[str] = None) -> None:
if error_message:
self.error_message = error_message
super().__init__(self.error_message)

@classmethod
def get_subclasses(cls) -> List[type["ConversionException"]]:
subclasses = [cls]
for subclass in cls.__subclasses__():
subclasses += subclass.get_subclasses()
return subclasses


class DocFormatUnsupported(ConversionException):
error_code = 10
error_message = "The document format is not supported"


class DocFormatUnsupportedHWPArm(DocFormatUnsupported):
error_code = 15
error_message = "HWP / HWPX formats are not supported in ARM architectures"


class DocFormatUnsupportedHWPQubes(DocFormatUnsupported):
error_code = 16
error_message = "HWP / HWPX formats are not supported in Qubes"


class LibreofficeFailure(ConversionException):
error_code = 20
error_message = "Conversion to PDF with LibreOffice failed"


class InvalidGMConversion(ConversionException):
error_code = 30
error_message = "Invalid conversion (Graphics Magic)"

def __init__(self, error_message: str) -> None:
super(error_message)


class PagesException(ConversionException):
error_code = 40


class NoPageCountException(PagesException):
error_code = 41
error_message = "Number of pages could not be extracted from PDF"


class PDFtoPPMException(ConversionException):
error_code = 50
error_message = f"Error converting PDF to Pixels (pdftoppm)"


class PDFtoPPMInvalidHeader(PDFtoPPMException):
error_code = 51
error_message = "Error converting PDF to Pixels (Invalid PPM header)"


class PDFtoPPMInvalidDepth(PDFtoPPMException):
error_code = 52
error_message = "Error converting PDF to Pixels (Invalid PPM depth)"


def exception_from_error_code(error_code: int) -> Optional[ConversionException]:
"""returns the conversion exception corresponding to the error code"""
for cls in ConversionException.get_subclasses():
if cls.error_code == error_code:
return cls()
raise ValueError(f"Unknown error code '{error_code}'")
6 changes: 5 additions & 1 deletion dangerzone/isolation_provider/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from colorama import Fore, Style

from ..conversion.errors import ConversionException
from ..document import Document
from ..util import replace_control_chars

Expand Down Expand Up @@ -36,7 +37,10 @@ def convert(
document.mark_as_converting()
try:
success = self._convert(document, ocr_lang)
except Exception:
except ConversionException as e:
success = False
self.print_progress_trusted(document, True, e.error_message, 0)
except Exception as e:
success = False
log.exception(
f"An exception occurred while converting document '{document.id}'"
Expand Down
4 changes: 4 additions & 0 deletions dangerzone/isolation_provider/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import tempfile
from typing import Any, Callable, List, Optional, Tuple

from ..conversion.errors import exception_from_error_code
from ..document import Document
from ..util import (
get_resource_path,
Expand Down Expand Up @@ -305,6 +306,9 @@ def _convert_with_tmpdirs(

if ret != 0:
log.error("documents-to-pixels failed")
raise exception_from_error_code(
ret
) # XXX Reconstruct exception from error code
else:
# TODO: validate convert to pixels output

Expand Down
10 changes: 9 additions & 1 deletion dangerzone/isolation_provider/qubes.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
log = logging.getLogger(__name__)

from ..conversion.common import running_on_qubes
from ..conversion.errors import exception_from_error_code
from ..conversion.pixels_to_pdf import PixelsToPDF
from ..util import get_resource_path, get_subprocess_startupinfo, get_tmp_dir
from .base import MAX_CONVERSION_LOG_CHARS
Expand All @@ -38,6 +39,8 @@ def read_bytes(p: subprocess.Popen, buff_size: int) -> bytes:
def read_int(p: subprocess.Popen) -> int:
"""Read 2 bytes from stdout, and decode them as int."""
untrusted_int = p.stdout.read(2) # type: ignore [union-attr]
if untrusted_int == b"":
raise ValueError("Nothing read from stdout (expected an integer)")
return int.from_bytes(untrusted_int, signed=False)


Expand Down Expand Up @@ -104,7 +107,12 @@ def _convert(
stderr=subprocess.DEVNULL,
)

n_pages = read_int(p)
try:
n_pages = read_int(p)
except ValueError:
error_code = p.wait()
# XXX Reconstruct exception from error code
raise exception_from_error_code(error_code) # type: ignore [misc]
if n_pages == 0:
# FIXME: Fail loudly in that case
return False
Expand Down

0 comments on commit 4c49d00

Please sign in to comment.