Skip to content

Commit be5c363

Browse files
committed
Fix markup injection issues
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.
1 parent a3e4d17 commit be5c363

File tree

12 files changed

+107
-73
lines changed

12 files changed

+107
-73
lines changed

qubes_config/global_config/rule_list_widgets.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ def _combobox_changed(self, *_args):
240240
self.callback()
241241

242242
def _format_new_value(self, new_value):
243-
self.name_widget.set_markup(f'{self.choices[new_value]}')
243+
self.name_widget.set_text(f'{self.choices[new_value]}')
244244
if self.verb_description:
245245
self.additional_text_widget.set_text(
246246
self.verb_description.get_verb_for_action_and_target(

qubes_config/global_config/thisdevice_handler.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,14 @@
2323

2424
import qubesadmin.vm
2525
from ..widgets.gtk_utils import show_error, load_icon, copy_to_global_clipboard
26+
from ..widgets.gtk_utils import markup_format
2627
from .page_handler import PageHandler
2728
from .policy_manager import PolicyManager
2829

2930
import gi
3031

3132
gi.require_version('Gtk', '3.0')
32-
from gi.repository import Gtk
33+
from gi.repository import Gtk, GLib
3334

3435
import gettext
3536
t = gettext.translation("desktop-linux-manager", fallback=True)
@@ -101,7 +102,7 @@ def __init__(self,
101102
['qubes-hcl-report', '-y']).decode()
102103
except subprocess.CalledProcessError as ex:
103104
label_text += _("Failed to load system data: {ex}\n").format(
104-
ex=str(ex))
105+
ex=GLib.markup_escape_text(str(ex)))
105106
self.hcl_check = ""
106107

107108
try:
@@ -115,7 +116,7 @@ def __init__(self,
115116
label_text += _("Failed to load system data.\n")
116117
self.data_label.get_style_context().add_class('red_code')
117118

118-
label_text += _("""<b>Brand:</b> {brand}
119+
label_text += markup_format("""<b>Brand:</b> {brand}
119120
<b>Model:</b> {model}
120121
121122
<b>CPU:</b> {cpu}
@@ -128,7 +129,8 @@ def __init__(self,
128129
<b>BIOS:</b> {bios}
129130
<b>Kernel:</b> {kernel_ver}
130131
<b>Xen:</b> {xen_ver}
131-
""").format(brand=self._get_data('brand'),
132+
""",
133+
brand=self._get_data('brand'),
132134
model=self._get_data('model'),
133135
cpu=self._get_data('cpu'),
134136
chipset=self._get_data('chipset'),
@@ -139,15 +141,15 @@ def __init__(self,
139141
kernel_ver=self._get_version('kernel'),
140142
xen_ver=self._get_version('xen'))
141143
self.set_state(self.compat_hvm_image, self._get_data('hvm'))
142-
self.compat_hvm_label.set_markup(f"<b>HVM:</b> {self._get_data('hvm')}")
144+
self.compat_hvm_label.set_markup(markup_format("<b>HVM:</b> {}", self._get_data('hvm')))
143145

144146
self.set_state(self.compat_iommu_image, self._get_data('iommu'))
145147
self.compat_iommu_label.set_markup(
146-
f"<b>I/O MMU:</b> {self._get_data('iommu')}")
148+
markup_format("<b>I/O MMU:</b> {}", self._get_data('iommu')))
147149

148150
self.set_state(self.compat_hap_image, self._get_data('slat'))
149151
self.compat_hap_label.set_markup(
150-
f"<b>HAP/SLAT:</b> {self._get_data('slat')}")
152+
markup_format("<b>HAP/SLAT:</b> {}", self._get_data('slat')))
151153

152154
self.set_state(self.compat_tpm_image,
153155
'yes' if self._get_data('tpm') == '1.2' else 'maybe')
@@ -166,7 +168,7 @@ def __init__(self,
166168

167169
self.set_state(self.compat_remapping_image, self._get_data('remap'))
168170
self.compat_remapping_label.set_markup(
169-
f"<b>Remapping:</b> {self._get_data('remap')}")
171+
markup_format(_("<b>Remapping:</b> {}"), self._get_data('remap')))
170172

171173
self.set_policy_state()
172174

@@ -178,7 +180,7 @@ def __init__(self,
178180
_("<b>PV qubes:</b> {num_pvs} found").format(num_pvs=len(pv_vms)))
179181
self.compat_pv_tooltip.set_tooltip_markup(
180182
_("<b>The following qubes have PV virtualization mode:</b>\n - ") +
181-
'\n - '.join([vm.name for vm in pv_vms]))
183+
'\n - '.join([GLib.markup_escape_text(vm.name) for vm in pv_vms]))
182184
self.compat_pv_tooltip.set_visible(bool(pv_vms))
183185

184186
self.data_label.set_markup(label_text)

qubes_config/global_config/usb_devices.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ def load_rules_for_usb_qube(self):
488488
def disable_u2f(self, reason: str):
489489
self.problem_fatal_box.set_visible(True)
490490
self.problem_fatal_box.show_all()
491-
self.problem_fatal_label.set_markup(reason)
491+
self.problem_fatal_label.set_text(reason)
492492
self.enable_check.set_active(False)
493493
self.enable_check.set_sensitive(False)
494494
self.box.set_visible(False)

qubes_config/widgets/gtk_utils.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,18 @@ def load_icon(icon_name: str, width: int = 24, height: int = 24):
8484
pixbuf.fill(0x000)
8585
return pixbuf
8686

87+
def _escape_str(s: Union[str, float, int]) -> Union[str, float, int]:
88+
if type(s) is str:
89+
return GLib.markup_escape_text(s)
90+
elif type(s) in (float, int, bool):
91+
return s
92+
else: # Neither escapable nor known safe to passthrough
93+
raise TypeError(f"Unsupported input type {type(s)}")
94+
95+
def markup_format(s, *args, **kwargs) -> str:
96+
escaped_args = [_escape_str(i) for i in args]
97+
escaped_kwargs = {k: _escape_str(v) for k, v in kwargs.items()}
98+
return s.format(*escaped_args, **escaped_kwargs)
8799

88100
def show_error(parent, title, text):
89101
"""
@@ -175,7 +187,7 @@ def show_dialog(
175187

176188
if isinstance(text, str):
177189
label: Gtk.Label = Gtk.Label()
178-
label.set_markup(text)
190+
label.set_text(text)
179191
label.set_line_wrap_mode(Gtk.WrapMode.WORD)
180192
label.set_max_width_chars(200)
181193
label.set_xalign(0)

qui/clipboard.py

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import gi
3939

4040
gi.require_version("Gtk", "3.0") # isort:skip
41-
from gi.repository import Gtk, Gio, Gdk # isort:skip
41+
from gi.repository import Gtk, Gio, Gdk, GLib # isort:skip
4242

4343
import gbulb
4444
import pyinotify
@@ -48,7 +48,7 @@
4848
t = gettext.translation("desktop-linux-manager", fallback=True)
4949
_ = t.gettext
5050

51-
from .utils import run_asyncio_and_show_errors
51+
from .utils import run_asyncio_and_show_errors, markup_format
5252

5353
gbulb.install()
5454

@@ -127,19 +127,19 @@ def _copy(self, metadata: dict) -> None:
127127
size = clipboard_formatted_size(metadata["sent_size"])
128128

129129
if metadata["malformed_request"]:
130-
body = ERROR_MALFORMED_DATA.format(vmname=metadata["vmname"])
130+
body = markup_format(ERROR_MALFORMED_DATA, vmname=metadata["vmname"])
131131
icon = "dialog-error"
132132
elif (
133133
metadata["qrexec_clipboard"]
134134
and metadata["sent_size"] >= metadata["buffer_size"]
135135
):
136136
# Microsoft Windows clipboard case
137-
body = WARNING_POSSIBLE_TRUNCATION.format(
137+
body = markup_format(WARNING_POSSIBLE_TRUNCATION,
138138
vmname=metadata["vmname"], size=size
139139
)
140140
icon = "dialog-warning"
141141
elif metadata["oversized_request"]:
142-
body = ERROR_OVERSIZED_DATA.format(
142+
body = markup_format(ERROR_OVERSIZED_DATA,
143143
vmname=metadata["vmname"],
144144
size=size,
145145
limit=clipboard_formatted_size(metadata["buffer_size"]),
@@ -150,13 +150,13 @@ def _copy(self, metadata: dict) -> None:
150150
and metadata["cleared"]
151151
and metadata["sent_size"] == 0
152152
):
153-
body = WARNING_EMPTY_CLIPBOARD.format(vmname=metadata["vmname"])
153+
body = markup_format(WARNING_EMPTY_CLIPBOARD, vmname=metadata["vmname"])
154154
icon = "dialog-warning"
155155
elif not metadata["successful"]:
156-
body = ERROR_ON_COPY.format(vmname=metadata["vmname"])
156+
body = markup_format(ERROR_ON_COPY, vmname=metadata["vmname"])
157157
icon = "dialog-error"
158158
else:
159-
body = MSG_COPY_SUCCESS.format(
159+
body = markup_format(MSG_COPY_SUCCESS,
160160
vmname=metadata["vmname"],
161161
size=size,
162162
shortcut=self.gtk_app.paste_shortcut,
@@ -173,14 +173,14 @@ def _copy(self, metadata: dict) -> None:
173173
def _paste(self, metadata: dict) -> None:
174174
"""Sends Paste notification via Gio.Notification."""
175175
if not metadata["successful"] or metadata["malformed_request"]:
176-
body = ERROR_ON_PASTE.format(vmname=metadata["vmname"])
176+
body = markup_format(ERROR_ON_PASTE, vmname=metadata["vmname"])
177177
body += MSG_WIPED
178178
icon = "dialog-error"
179179
elif (
180180
"protocol_version_xside" in metadata.keys()
181181
and metadata["protocol_version_xside"] >= 0x00010008
182182
):
183-
body = MSG_PASTE_SUCCESS_METADATA.format(
183+
body = markup_format(MSG_PASTE_SUCCESS_METADATA,
184184
size=clipboard_formatted_size(metadata["sent_size"]),
185185
vmname=metadata["vmname"],
186186
)
@@ -355,9 +355,9 @@ def update_clipboard_contents(
355355

356356
else:
357357
self.clipboard_label.set_markup(
358-
_(
358+
markup_format(_(
359359
"<i>Global clipboard contents: {0} from <b>{1}</b></i>"
360-
).format(size, vm)
360+
), size, vm)
361361
)
362362
self.icon.set_from_icon_name("edit-copy")
363363

@@ -391,10 +391,10 @@ def setup_ui(self, *_args, **_kwargs):
391391

392392
help_label = Gtk.Label(xalign=0)
393393
help_label.set_markup(
394-
_(
394+
markup_format(_(
395395
"<i>Use <b>{copy}</b> to copy and "
396396
"<b>{paste}</b> to paste.</i>"
397-
).format(copy=self.copy_shortcut, paste=self.paste_shortcut)
397+
), copy=self.copy_shortcut, paste=self.paste_shortcut)
398398
)
399399
help_item = Gtk.MenuItem()
400400
help_item.set_margin_left(10)
@@ -442,9 +442,9 @@ def copy_dom0_clipboard(self, *_args, **_kwargs):
442442
'"protocol_version_xside":65544,\n'
443443
'"protocol_version_vmside":65544,\n'
444444
"}}\n".format(
445-
xevent_timestamp=str(Gtk.get_current_event_time()),
446-
sent_size=os.path.getsize(DATA),
447-
buffer_size="256000",
445+
xevent_timestamp=json.dumps(Gtk.get_current_event_time()),
446+
sent_size=json.dumps(os.path.getsize(DATA)),
447+
buffer_size=json.dumps(256000),
448448
)
449449
)
450450
except Exception: # pylint: disable=broad-except

qui/decorators.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from gi.repository import Gtk, Pango, GLib, GdkPixbuf # isort:skip
1010
from qubesadmin import exc
1111
from qubesadmin.utils import size_to_human
12+
from .utils import markup_format
1213

1314
import gettext
1415
t = gettext.translation("desktop-linux-manager", fallback=True)
@@ -137,10 +138,10 @@ def update_tooltip(self,
137138
perc_storage = self.cur_storage / self.max_storage
138139

139140
tooltip += \
140-
_("\nTemplate: <b>{template}</b>"
141+
markup_format(_("\nTemplate: <b>{template}</b>"
141142
"\nNetworking: <b>{netvm}</b>"
142143
"\nPrivate storage: <b>{current_storage:.2f}GB/"
143-
"{max_storage:.2f}GB ({perc_storage:.1%})</b>").format(
144+
"{max_storage:.2f}GB ({perc_storage:.1%})</b>"),
144145
template=self.template_name,
145146
netvm=self.netvm_name,
146147
current_storage=self.cur_storage,
@@ -177,7 +178,8 @@ def update_state(self, cpu=0, header=False):
177178
else:
178179
color = self.cpu_label.get_style_context() \
179180
.get_color(Gtk.StateFlags.INSENSITIVE).to_color()
180-
markup = f'<span color="{color.to_string()}">0%</span>'
181+
escaped_color = GLib.markup_escape_text(color.to_string())
182+
markup = f'<span color="{escaped_color}">0%</span>'
181183

182184
self.cpu_label.set_markup(markup)
183185

@@ -249,8 +251,9 @@ def device_hbox(device) -> Gtk.Box:
249251
name_label = Gtk.Label(xalign=0)
250252
name = f"{device.backend_domain}:{device.port_id} - {device.description}"
251253
if device.attachments:
252-
dev_list = ", ".join(list(device.attachments))
253-
name_label.set_markup(f'<b>{name} ({dev_list})</b>')
254+
dev_list = GLib.markup_escape_text(", ".join(list(device.attachments)))
255+
name_escaped = GLib.markup_escape_text(name)
256+
name_label.set_markup(f'<b>{name_escaped} ({dev_list})</b>')
254257
else:
255258
name_label.set_text(name)
256259
name_label.set_max_width_chars(64)
@@ -281,7 +284,7 @@ def device_domain_hbox(vm, attached: bool) -> Gtk.Box:
281284

282285
name = Gtk.Label(xalign=0)
283286
if attached:
284-
name.set_markup(f'<b>{vm.vm_name}</b>')
287+
name.set_markup(f'<b>{GLib.markup_escape_text(vm.vm_name)}</b>')
285288
else:
286289
name.set_text(vm.vm_name)
287290

qui/devices/actionable_widgets.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ def __init__(self, vm: backend.VM, size: int = 18, variant: str = 'dark',
130130
self.backend_label = Gtk.Label(xalign=0)
131131
backend_label: str = vm.name
132132
if name_extension:
133-
backend_label += ": " + name_extension
133+
backend_label += ": " + GLib.markup_escape_text(name_extension)
134134
self.backend_label.set_markup(backend_label)
135135

136136
self.pack_start(self.backend_icon, False, False, 4)
@@ -227,7 +227,7 @@ class DetachWidget(ActionableWidget, SimpleActionWidget):
227227
"""Detach device from a VM"""
228228
def __init__(self, vm: backend.VM, device: backend.Device,
229229
variant: str = 'dark'):
230-
super().__init__('detach', '<b>Detach from ' + vm.name + '</b>',
230+
super().__init__('detach', '<b>Detach from ' + GLib.markup_escape_text(vm.name) + '</b>',
231231
variant)
232232
self.vm = vm
233233
self.device = device
@@ -345,14 +345,14 @@ def __init__(self, device: backend.Device, variant: str = 'dark'):
345345
super().__init__(orientation=Gtk.Orientation.VERTICAL)
346346
# FUTURE: this is proposed layout for new API
347347
# self.device_label = Gtk.Label()
348-
# self.device_label.set_markup(device.name)
348+
# self.device_label.set_text(device.name)
349349
# self.device_label.get_style_context().add_class('device_name')
350350
# self.edit_icon = VariantIcon('edit', 'dark', 24)
351351
# self.detailed_description_label = Gtk.Label()
352352
# self.detailed_description_label.set_text(device.description)
353353
# self.backend_icon = VariantIcon(device.vm_icon, 'dark', 24)
354354
# self.backend_label = Gtk.Label(xalign=0)
355-
# self.backend_label.set_markup(str(device.backend_domain))
355+
# self.backend_label.set_text(str(device.backend_domain))
356356
#
357357
# self.title_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)
358358
# self.title_box.add(self.device_label)
@@ -367,7 +367,7 @@ def __init__(self, device: backend.Device, variant: str = 'dark'):
367367
# self.add(self.attachment_box)
368368

369369
self.device_label = Gtk.Label()
370-
self.device_label.set_markup(device.name)
370+
self.device_label.set_text(device.name)
371371
self.device_label.get_style_context().add_class('device_name')
372372
self.device_label.set_xalign(Gtk.Align.CENTER)
373373
self.device_label.set_halign(Gtk.Align.CENTER)
@@ -408,7 +408,7 @@ def __init__(self, device: backend.Device, variant: str = 'dark'):
408408

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

411-
label_markup = device.name
411+
label_markup = GLib.markup_escape_text(device.name)
412412
if (device.connection_timestamp and
413413
int(time.monotonic() - device.connection_timestamp) < 120):
414414
label_markup += ' <span foreground="#63a0ff"><b>NEW</b></span>'

0 commit comments

Comments
 (0)