Skip to content

Commit

Permalink
Fix markup injection issues
Browse files Browse the repository at this point in the history
This fixes some (theoretical) markup injection problems.  I believe none
of the strings that I escape here will ever contain "<" or "&", but it
is always safer to escape.
  • Loading branch information
DemiMarie committed Jan 6, 2025
1 parent 1231efc commit 275d9f6
Show file tree
Hide file tree
Showing 11 changed files with 185 additions and 87 deletions.
2 changes: 1 addition & 1 deletion qubes_config/global_config/rule_list_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def _combobox_changed(self, *_args):
self.callback()

def _format_new_value(self, new_value):
self.name_widget.set_markup(f"{self.choices[new_value]}")
self.name_widget.set_text(f"{self.choices[new_value]}")
if self.verb_description:
self.additional_text_widget.set_text(
self.verb_description.get_verb_for_action_and_target(
Expand Down
24 changes: 14 additions & 10 deletions qubes_config/global_config/thisdevice_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@

import qubesadmin.vm
from ..widgets.gtk_utils import show_error, load_icon, copy_to_global_clipboard
from ..widgets.gtk_utils import markup_format
from .page_handler import PageHandler
from .policy_manager import PolicyManager

import gi

gi.require_version("Gtk", "3.0")
from gi.repository import Gtk
from gi.repository import Gtk, GLib

import gettext

Expand Down Expand Up @@ -126,7 +127,7 @@ def __init__(
).decode()
except subprocess.CalledProcessError as ex:
label_text += _("Failed to load system data: {ex}\n").format(
ex=str(ex)
ex=GLib.markup_escape_text(str(ex))
)
self.hcl_check = ""

Expand All @@ -141,8 +142,9 @@ def __init__(
label_text += _("Failed to load system data.\n")
self.data_label.get_style_context().add_class("red_code")

label_text += _(
"""<b>Brand:</b> {brand}
label_text += markup_format(
_(
"""<b>Brand:</b> {brand}
<b>Model:</b> {model}
<b>CPU:</b> {cpu}
Expand All @@ -156,7 +158,7 @@ def __init__(
<b>Kernel:</b> {kernel_ver}
<b>Xen:</b> {xen_ver}
"""
).format(
),
brand=self._get_data("brand"),
model=self._get_data("model"),
cpu=self._get_data("cpu"),
Expand All @@ -169,16 +171,18 @@ def __init__(
xen_ver=self._get_version("xen"),
)
self.set_state(self.compat_hvm_image, self._get_data("hvm"))
self.compat_hvm_label.set_markup(f"<b>HVM:</b> {self._get_data('hvm')}")
self.compat_hvm_label.set_markup(
markup_format("<b>HVM:</b> {}", self._get_data("hvm"))
)

self.set_state(self.compat_iommu_image, self._get_data("iommu"))
self.compat_iommu_label.set_markup(
f"<b>I/O MMU:</b> {self._get_data('iommu')}"
markup_format("<b>I/O MMU:</b> {}", self._get_data("iommu"))
)

self.set_state(self.compat_hap_image, self._get_data("slat"))
self.compat_hap_label.set_markup(
f"<b>HAP/SLAT:</b> {self._get_data('slat')}"
markup_format("<b>HAP/SLAT:</b> {}", self._get_data("slat"))
)

self.set_state(
Expand All @@ -201,7 +205,7 @@ def __init__(

self.set_state(self.compat_remapping_image, self._get_data("remap"))
self.compat_remapping_label.set_markup(
f"<b>Remapping:</b> {self._get_data('remap')}"
markup_format(_("<b>Remapping:</b> {}"), self._get_data("remap"))
)

self.set_policy_state()
Expand All @@ -218,7 +222,7 @@ def __init__(
)
self.compat_pv_tooltip.set_tooltip_markup(
_("<b>The following qubes have PV virtualization mode:</b>\n - ")
+ "\n - ".join([vm.name for vm in pv_vms])
+ "\n - ".join([GLib.markup_escape_text(vm.name) for vm in pv_vms])
)
self.compat_pv_tooltip.set_visible(bool(pv_vms))

Expand Down
2 changes: 1 addition & 1 deletion qubes_config/global_config/usb_devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ def load_rules_for_usb_qube(self):
def disable_u2f(self, reason: str):
self.problem_fatal_box.set_visible(True)
self.problem_fatal_box.show_all()
self.problem_fatal_label.set_markup(reason)
self.problem_fatal_label.set_text(reason)
self.enable_check.set_active(False)
self.enable_check.set_sensitive(False)
self.box.set_visible(False)
Expand Down
18 changes: 17 additions & 1 deletion qubes_config/widgets/gtk_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ def load_icon(icon_name: str, width: int = 24, height: int = 24):
return pixbuf


def _escape_str(s: Union[str, float, int]) -> Union[str, float, int]:
# pylint: disable=unidiomatic-typecheck
if type(s) is str:
return GLib.markup_escape_text(s)
# pylint: disable=unidiomatic-typecheck
if type(s) in (float, int, bool):
return s
raise TypeError(f"Unsupported input type {type(s)}")


def markup_format(s, *args, **kwargs) -> str:
escaped_args = [_escape_str(i) for i in args]
escaped_kwargs = {k: _escape_str(v) for k, v in kwargs.items()}
return s.format(*escaped_args, **escaped_kwargs)


def show_error(parent, title, text):
"""
Helper function to display error messages.
Expand Down Expand Up @@ -184,7 +200,7 @@ def show_dialog(

if isinstance(text, str):
label: Gtk.Label = Gtk.Label()
label.set_markup(text)
label.set_text(text)
label.set_line_wrap_mode(Gtk.WrapMode.WORD)
label.set_max_width_chars(200)
label.set_xalign(0)
Expand Down
57 changes: 37 additions & 20 deletions qui/clipboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
t = gettext.translation("desktop-linux-manager", fallback=True)
_ = t.gettext

from .utils import run_asyncio_and_show_errors
from .utils import run_asyncio_and_show_errors, markup_format

gbulb.install()

Expand Down Expand Up @@ -127,19 +127,24 @@ def _copy(self, metadata: dict) -> None:
size = clipboard_formatted_size(metadata["sent_size"])

if metadata["malformed_request"]:
body = ERROR_MALFORMED_DATA.format(vmname=metadata["vmname"])
body = markup_format(
ERROR_MALFORMED_DATA, vmname=metadata["vmname"]
)
icon = "dialog-error"
elif (
metadata["qrexec_clipboard"]
and metadata["sent_size"] >= metadata["buffer_size"]
):
# Microsoft Windows clipboard case
body = WARNING_POSSIBLE_TRUNCATION.format(
vmname=metadata["vmname"], size=size
body = markup_format(
WARNING_POSSIBLE_TRUNCATION,
vmname=metadata["vmname"],
size=size,
)
icon = "dialog-warning"
elif metadata["oversized_request"]:
body = ERROR_OVERSIZED_DATA.format(
body = markup_format(
ERROR_OVERSIZED_DATA,
vmname=metadata["vmname"],
size=size,
limit=clipboard_formatted_size(metadata["buffer_size"]),
Expand All @@ -150,13 +155,16 @@ def _copy(self, metadata: dict) -> None:
and metadata["cleared"]
and metadata["sent_size"] == 0
):
body = WARNING_EMPTY_CLIPBOARD.format(vmname=metadata["vmname"])
body = markup_format(
WARNING_EMPTY_CLIPBOARD, vmname=metadata["vmname"]
)
icon = "dialog-warning"
elif not metadata["successful"]:
body = ERROR_ON_COPY.format(vmname=metadata["vmname"])
body = markup_format(ERROR_ON_COPY, vmname=metadata["vmname"])
icon = "dialog-error"
else:
body = MSG_COPY_SUCCESS.format(
body = markup_format(
MSG_COPY_SUCCESS,
vmname=metadata["vmname"],
size=size,
shortcut=self.gtk_app.paste_shortcut,
Expand All @@ -173,14 +181,15 @@ def _copy(self, metadata: dict) -> None:
def _paste(self, metadata: dict) -> None:
"""Sends Paste notification via Gio.Notification."""
if not metadata["successful"] or metadata["malformed_request"]:
body = ERROR_ON_PASTE.format(vmname=metadata["vmname"])
body = markup_format(ERROR_ON_PASTE, vmname=metadata["vmname"])
body += MSG_WIPED
icon = "dialog-error"
elif (
"protocol_version_xside" in metadata.keys()
and metadata["protocol_version_xside"] >= 0x00010008
):
body = MSG_PASTE_SUCCESS_METADATA.format(
body = markup_format(
MSG_PASTE_SUCCESS_METADATA,
size=clipboard_formatted_size(metadata["sent_size"]),
vmname=metadata["vmname"],
)
Expand Down Expand Up @@ -355,9 +364,11 @@ def update_clipboard_contents(

else:
self.clipboard_label.set_markup(
_(
"<i>Global clipboard contents: {0} from <b>{1}</b></i>"
).format(size, vm)
markup_format(
_("<i>Global clipboard contents: {0} from <b>{1}</b></i>"),
size,
vm,
)
)
self.icon.set_from_icon_name("edit-copy")

Expand Down Expand Up @@ -391,10 +402,14 @@ def setup_ui(self, *_args, **_kwargs):

help_label = Gtk.Label(xalign=0)
help_label.set_markup(
_(
"<i>Use <b>{copy}</b> to copy and "
"<b>{paste}</b> to paste.</i>"
).format(copy=self.copy_shortcut, paste=self.paste_shortcut)
markup_format(
_(
"<i>Use <b>{copy}</b> to copy and "
"<b>{paste}</b> to paste.</i>"
),
copy=self.copy_shortcut,
paste=self.paste_shortcut,
)
)
help_item = Gtk.MenuItem()
help_item.set_margin_left(10)
Expand Down Expand Up @@ -442,9 +457,11 @@ def copy_dom0_clipboard(self, *_args, **_kwargs):
'"protocol_version_xside":65544,\n'
'"protocol_version_vmside":65544,\n'
"}}\n".format(
xevent_timestamp=str(Gtk.get_current_event_time()),
sent_size=os.path.getsize(DATA),
buffer_size="256000",
xevent_timestamp=json.dumps(
Gtk.get_current_event_time()
),
sent_size=json.dumps(os.path.getsize(DATA)),
buffer_size=json.dumps(256000),
)
)
except Exception: # pylint: disable=broad-except
Expand Down
24 changes: 14 additions & 10 deletions qui/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from gi.repository import Gtk, Pango, GLib, GdkPixbuf # isort:skip
from qubesadmin import exc
from qubesadmin.utils import size_to_human
from .utils import markup_format

import gettext

Expand Down Expand Up @@ -146,12 +147,13 @@ def update_tooltip(self, netvm_changed=False, storage_changed=False):
else:
perc_storage = self.cur_storage / self.max_storage

tooltip += _(
"\nTemplate: <b>{template}</b>"
"\nNetworking: <b>{netvm}</b>"
"\nPrivate storage: <b>{current_storage:.2f}GB/"
"{max_storage:.2f}GB ({perc_storage:.1%})</b>"
).format(
tooltip += markup_format(
_(
"\nTemplate: <b>{template}</b>"
"\nNetworking: <b>{netvm}</b>"
"\nPrivate storage: <b>{current_storage:.2f}GB/"
"{max_storage:.2f}GB ({perc_storage:.1%})</b>"
),
template=self.template_name,
netvm=self.netvm_name,
current_storage=self.cur_storage,
Expand Down Expand Up @@ -193,7 +195,8 @@ def update_state(self, cpu=0, header=False):
.get_color(Gtk.StateFlags.INSENSITIVE)
.to_color()
)
markup = f'<span color="{color.to_string()}">0%</span>'
escaped_color = GLib.markup_escape_text(color.to_string())
markup = f'<span color="{escaped_color}">0%</span>'

self.cpu_label.set_markup(markup)

Expand Down Expand Up @@ -264,8 +267,9 @@ def device_hbox(device) -> Gtk.Box:
name_label = Gtk.Label(xalign=0)
name = f"{device.backend_domain}:{device.port_id} - {device.description}"
if device.attachments:
dev_list = ", ".join(list(device.attachments))
name_label.set_markup(f"<b>{name} ({dev_list})</b>")
dev_list = GLib.markup_escape_text(", ".join(list(device.attachments)))
name_escaped = GLib.markup_escape_text(name)
name_label.set_markup(f"<b>{name_escaped} ({dev_list})</b>")
else:
name_label.set_text(name)
name_label.set_max_width_chars(64)
Expand Down Expand Up @@ -296,7 +300,7 @@ def device_domain_hbox(vm, attached: bool) -> Gtk.Box:

name = Gtk.Label(xalign=0)
if attached:
name.set_markup(f"<b>{vm.vm_name}</b>")
name.set_markup(f"<b>{GLib.markup_escape_text(vm.vm_name)}</b>")
else:
name.set_text(vm.vm_name)

Expand Down
14 changes: 8 additions & 6 deletions qui/devices/actionable_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def __init__(
self.backend_label = Gtk.Label(xalign=0)
backend_label: str = vm.name
if name_extension:
backend_label += ": " + name_extension
backend_label += ": " + GLib.markup_escape_text(name_extension)
self.backend_label.set_markup(backend_label)

self.pack_start(self.backend_icon, False, False, 4)
Expand Down Expand Up @@ -250,7 +250,9 @@ def __init__(
self, vm: backend.VM, device: backend.Device, variant: str = "dark"
):
super().__init__(
"detach", "<b>Detach from " + vm.name + "</b>", variant
"detach",
"<b>Detach from " + GLib.markup_escape_text(vm.name) + "</b>",
variant,
)
self.vm = vm
self.device = device
Expand Down Expand Up @@ -383,14 +385,14 @@ def __init__(self, device: backend.Device, variant: str = "dark"):
super().__init__(orientation=Gtk.Orientation.VERTICAL)
# FUTURE: this is proposed layout for new API
# self.device_label = Gtk.Label()
# self.device_label.set_markup(device.name)
# self.device_label.set_text(device.name)
# self.device_label.get_style_context().add_class('device_name')
# self.edit_icon = VariantIcon('edit', 'dark', 24)
# self.detailed_description_label = Gtk.Label()
# self.detailed_description_label.set_text(device.description)
# self.backend_icon = VariantIcon(device.vm_icon, 'dark', 24)
# self.backend_label = Gtk.Label(xalign=0)
# self.backend_label.set_markup(str(device.backend_domain))
# self.backend_label.set_text(str(device.backend_domain))
#
# self.title_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)
# self.title_box.add(self.device_label)
Expand All @@ -405,7 +407,7 @@ def __init__(self, device: backend.Device, variant: str = "dark"):
# self.add(self.attachment_box)

self.device_label = Gtk.Label()
self.device_label.set_markup(device.name)
self.device_label.set_text(device.name)
self.device_label.get_style_context().add_class("device_name")
self.device_label.set_xalign(Gtk.Align.CENTER)
self.device_label.set_halign(Gtk.Align.CENTER)
Expand Down Expand Up @@ -447,7 +449,7 @@ def __init__(self, device: backend.Device, variant: str = "dark"):

self.device_label = Gtk.Label(xalign=0)

label_markup = device.name
label_markup = GLib.markup_escape_text(device.name)
if (
device.connection_timestamp
and int(time.monotonic() - device.connection_timestamp) < 120
Expand Down
Loading

0 comments on commit 275d9f6

Please sign in to comment.