Skip to content

Commit 99c6f83

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 e011e5f commit 99c6f83

File tree

11 files changed

+185
-87
lines changed

11 files changed

+185
-87
lines changed

qubes_config/global_config/rule_list_widgets.py

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

263263
def _format_new_value(self, new_value):
264-
self.name_widget.set_markup(f"{self.choices[new_value]}")
264+
self.name_widget.set_text(f"{self.choices[new_value]}")
265265
if self.verb_description:
266266
self.additional_text_widget.set_text(
267267
self.verb_description.get_verb_for_action_and_target(

qubes_config/global_config/thisdevice_handler.py

Lines changed: 14 additions & 10 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

@@ -126,7 +127,7 @@ def __init__(
126127
).decode()
127128
except subprocess.CalledProcessError as ex:
128129
label_text += _("Failed to load system data: {ex}\n").format(
129-
ex=str(ex)
130+
ex=GLib.markup_escape_text(str(ex))
130131
)
131132
self.hcl_check = ""
132133

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

144-
label_text += _(
145-
"""<b>Brand:</b> {brand}
145+
label_text += markup_format(
146+
_(
147+
"""<b>Brand:</b> {brand}
146148
<b>Model:</b> {model}
147149
148150
<b>CPU:</b> {cpu}
@@ -156,7 +158,7 @@ def __init__(
156158
<b>Kernel:</b> {kernel_ver}
157159
<b>Xen:</b> {xen_ver}
158160
"""
159-
).format(
161+
),
160162
brand=self._get_data("brand"),
161163
model=self._get_data("model"),
162164
cpu=self._get_data("cpu"),
@@ -169,16 +171,18 @@ def __init__(
169171
xen_ver=self._get_version("xen"),
170172
)
171173
self.set_state(self.compat_hvm_image, self._get_data("hvm"))
172-
self.compat_hvm_label.set_markup(f"<b>HVM:</b> {self._get_data('hvm')}")
174+
self.compat_hvm_label.set_markup(
175+
markup_format("<b>HVM:</b> {}", self._get_data("hvm"))
176+
)
173177

174178
self.set_state(self.compat_iommu_image, self._get_data("iommu"))
175179
self.compat_iommu_label.set_markup(
176-
f"<b>I/O MMU:</b> {self._get_data('iommu')}"
180+
markup_format("<b>I/O MMU:</b> {}", self._get_data("iommu"))
177181
)
178182

179183
self.set_state(self.compat_hap_image, self._get_data("slat"))
180184
self.compat_hap_label.set_markup(
181-
f"<b>HAP/SLAT:</b> {self._get_data('slat')}"
185+
markup_format("<b>HAP/SLAT:</b> {}", self._get_data("slat"))
182186
)
183187

184188
self.set_state(
@@ -201,7 +205,7 @@ def __init__(
201205

202206
self.set_state(self.compat_remapping_image, self._get_data("remap"))
203207
self.compat_remapping_label.set_markup(
204-
f"<b>Remapping:</b> {self._get_data('remap')}"
208+
markup_format(_("<b>Remapping:</b> {}"), self._get_data("remap"))
205209
)
206210

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

qubes_config/global_config/usb_devices.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ def load_rules_for_usb_qube(self):
554554
def disable_u2f(self, reason: str):
555555
self.problem_fatal_box.set_visible(True)
556556
self.problem_fatal_box.show_all()
557-
self.problem_fatal_label.set_markup(reason)
557+
self.problem_fatal_label.set_text(reason)
558558
self.enable_check.set_active(False)
559559
self.enable_check.set_sensitive(False)
560560
self.box.set_visible(False)

qubes_config/widgets/gtk_utils.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,22 @@ def load_icon(icon_name: str, width: int = 24, height: int = 24):
8888
return pixbuf
8989

9090

91+
def _escape_str(s: Union[str, float, int]) -> Union[str, float, int]:
92+
# pylint: disable=unidiomatic-typecheck
93+
if type(s) is str:
94+
return GLib.markup_escape_text(s)
95+
# pylint: disable=unidiomatic-typecheck
96+
if type(s) in (float, int, bool):
97+
return s
98+
raise TypeError(f"Unsupported input type {type(s)}")
99+
100+
101+
def markup_format(s, *args, **kwargs) -> str:
102+
escaped_args = [_escape_str(i) for i in args]
103+
escaped_kwargs = {k: _escape_str(v) for k, v in kwargs.items()}
104+
return s.format(*escaped_args, **escaped_kwargs)
105+
106+
91107
def show_error(parent, title, text):
92108
"""
93109
Helper function to display error messages.
@@ -184,7 +200,7 @@ def show_dialog(
184200

185201
if isinstance(text, str):
186202
label: Gtk.Label = Gtk.Label()
187-
label.set_markup(text)
203+
label.set_text(text)
188204
label.set_line_wrap_mode(Gtk.WrapMode.WORD)
189205
label.set_max_width_chars(200)
190206
label.set_xalign(0)

qui/clipboard.py

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
t = gettext.translation("desktop-linux-manager", fallback=True)
5454
_ = t.gettext
5555

56-
from .utils import run_asyncio_and_show_errors
56+
from .utils import run_asyncio_and_show_errors, markup_format
5757

5858
gbulb.install()
5959

@@ -132,19 +132,24 @@ def _copy(self, metadata: dict) -> None:
132132
size = clipboard_formatted_size(metadata["sent_size"])
133133

134134
if metadata["malformed_request"]:
135-
body = ERROR_MALFORMED_DATA.format(vmname=metadata["vmname"])
135+
body = markup_format(
136+
ERROR_MALFORMED_DATA, vmname=metadata["vmname"]
137+
)
136138
icon = "dialog-error"
137139
elif (
138140
metadata["qrexec_clipboard"]
139141
and metadata["sent_size"] >= metadata["buffer_size"]
140142
):
141143
# Microsoft Windows clipboard case
142-
body = WARNING_POSSIBLE_TRUNCATION.format(
143-
vmname=metadata["vmname"], size=size
144+
body = markup_format(
145+
WARNING_POSSIBLE_TRUNCATION,
146+
vmname=metadata["vmname"],
147+
size=size,
144148
)
145149
icon = "dialog-warning"
146150
elif metadata["oversized_request"]:
147-
body = ERROR_OVERSIZED_DATA.format(
151+
body = markup_format(
152+
ERROR_OVERSIZED_DATA,
148153
vmname=metadata["vmname"],
149154
size=size,
150155
limit=clipboard_formatted_size(metadata["buffer_size"]),
@@ -155,13 +160,16 @@ def _copy(self, metadata: dict) -> None:
155160
and metadata["cleared"]
156161
and metadata["sent_size"] == 0
157162
):
158-
body = WARNING_EMPTY_CLIPBOARD.format(vmname=metadata["vmname"])
163+
body = markup_format(
164+
WARNING_EMPTY_CLIPBOARD, vmname=metadata["vmname"]
165+
)
159166
icon = "dialog-warning"
160167
elif not metadata["successful"]:
161-
body = ERROR_ON_COPY.format(vmname=metadata["vmname"])
168+
body = markup_format(ERROR_ON_COPY, vmname=metadata["vmname"])
162169
icon = "dialog-error"
163170
else:
164-
body = MSG_COPY_SUCCESS.format(
171+
body = markup_format(
172+
MSG_COPY_SUCCESS,
165173
vmname=metadata["vmname"],
166174
size=size,
167175
shortcut=self.gtk_app.paste_shortcut,
@@ -178,14 +186,15 @@ def _copy(self, metadata: dict) -> None:
178186
def _paste(self, metadata: dict) -> None:
179187
"""Sends Paste notification via Gio.Notification."""
180188
if not metadata["successful"] or metadata["malformed_request"]:
181-
body = ERROR_ON_PASTE.format(vmname=metadata["vmname"])
189+
body = markup_format(ERROR_ON_PASTE, vmname=metadata["vmname"])
182190
body += MSG_WIPED
183191
icon = "dialog-error"
184192
elif (
185193
"protocol_version_xside" in metadata.keys()
186194
and metadata["protocol_version_xside"] >= 0x00010008
187195
):
188-
body = MSG_PASTE_SUCCESS_METADATA.format(
196+
body = markup_format(
197+
MSG_PASTE_SUCCESS_METADATA,
189198
size=clipboard_formatted_size(metadata["sent_size"]),
190199
vmname=metadata["vmname"],
191200
)
@@ -361,9 +370,11 @@ def update_clipboard_contents(
361370

362371
else:
363372
self.clipboard_label.set_markup(
364-
_(
365-
"<i>Global clipboard contents: {0} from <b>{1}</b></i>"
366-
).format(size, vm)
373+
markup_format(
374+
_("<i>Global clipboard contents: {0} from <b>{1}</b></i>"),
375+
size,
376+
vm,
377+
)
367378
)
368379
self.icon.set_from_icon_name("edit-copy")
369380

@@ -398,10 +409,14 @@ def setup_ui(self, *_args, **_kwargs):
398409

399410
help_label = Gtk.Label(xalign=0)
400411
help_label.set_markup(
401-
_(
402-
"<i>Use <b>{copy}</b> to copy and "
403-
"<b>{paste}</b> to paste.</i>"
404-
).format(copy=self.copy_shortcut, paste=self.paste_shortcut)
412+
markup_format(
413+
_(
414+
"<i>Use <b>{copy}</b> to copy and "
415+
"<b>{paste}</b> to paste.</i>"
416+
),
417+
copy=self.copy_shortcut,
418+
paste=self.paste_shortcut,
419+
)
405420
)
406421
help_item = Gtk.MenuItem()
407422
help_item.set_margin_left(10)
@@ -449,9 +464,11 @@ def copy_dom0_clipboard(self, *_args, **_kwargs):
449464
'"protocol_version_xside":65544,\n'
450465
'"protocol_version_vmside":65544,\n'
451466
"}}\n".format(
452-
xevent_timestamp=str(Gtk.get_current_event_time()),
453-
sent_size=os.path.getsize(DATA),
454-
buffer_size="256000",
467+
xevent_timestamp=json.dumps(
468+
Gtk.get_current_event_time()
469+
),
470+
sent_size=json.dumps(os.path.getsize(DATA)),
471+
buffer_size=json.dumps(256000),
455472
)
456473
)
457474
except Exception: # pylint: disable=broad-except

qui/decorators.py

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

1415
import gettext
1516

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

149-
tooltip += _(
150-
"\nTemplate: <b>{template}</b>"
151-
"\nNetworking: <b>{netvm}</b>"
152-
"\nPrivate storage: <b>{current_storage:.2f}GB/"
153-
"{max_storage:.2f}GB ({perc_storage:.1%})</b>"
154-
).format(
150+
tooltip += markup_format(
151+
_(
152+
"\nTemplate: <b>{template}</b>"
153+
"\nNetworking: <b>{netvm}</b>"
154+
"\nPrivate storage: <b>{current_storage:.2f}GB/"
155+
"{max_storage:.2f}GB ({perc_storage:.1%})</b>"
156+
),
155157
template=self.template_name,
156158
netvm=self.netvm_name,
157159
current_storage=self.cur_storage,
@@ -193,7 +195,8 @@ def update_state(self, cpu=0, header=False):
193195
.get_color(Gtk.StateFlags.INSENSITIVE)
194196
.to_color()
195197
)
196-
markup = f'<span color="{color.to_string()}">0%</span>'
198+
escaped_color = GLib.markup_escape_text(color.to_string())
199+
markup = f'<span color="{escaped_color}">0%</span>'
197200

198201
self.cpu_label.set_markup(markup)
199202

@@ -264,8 +267,9 @@ def device_hbox(device) -> Gtk.Box:
264267
name_label = Gtk.Label(xalign=0)
265268
name = f"{device.backend_domain}:{device.port_id} - {device.description}"
266269
if device.attachments:
267-
dev_list = ", ".join(list(device.attachments))
268-
name_label.set_markup(f"<b>{name} ({dev_list})</b>")
270+
dev_list = GLib.markup_escape_text(", ".join(list(device.attachments)))
271+
name_escaped = GLib.markup_escape_text(name)
272+
name_label.set_markup(f"<b>{name_escaped} ({dev_list})</b>")
269273
else:
270274
name_label.set_text(name)
271275
name_label.set_max_width_chars(64)
@@ -296,7 +300,7 @@ def device_domain_hbox(vm, attached: bool) -> Gtk.Box:
296300

297301
name = Gtk.Label(xalign=0)
298302
if attached:
299-
name.set_markup(f"<b>{vm.vm_name}</b>")
303+
name.set_markup(f"<b>{GLib.markup_escape_text(vm.vm_name)}</b>")
300304
else:
301305
name.set_text(vm.vm_name)
302306

qui/devices/actionable_widgets.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def __init__(
145145
self.backend_label = Gtk.Label(xalign=0)
146146
backend_label: str = vm.name
147147
if name_extension:
148-
backend_label += ": " + name_extension
148+
backend_label += ": " + GLib.markup_escape_text(name_extension)
149149
self.backend_label.set_markup(backend_label)
150150

151151
self.pack_start(self.backend_icon, False, False, 4)
@@ -250,7 +250,9 @@ def __init__(
250250
self, vm: backend.VM, device: backend.Device, variant: str = "dark"
251251
):
252252
super().__init__(
253-
"detach", "<b>Detach from " + vm.name + "</b>", variant
253+
"detach",
254+
"<b>Detach from " + GLib.markup_escape_text(vm.name) + "</b>",
255+
variant,
254256
)
255257
self.vm = vm
256258
self.device = device
@@ -383,14 +385,14 @@ def __init__(self, device: backend.Device, variant: str = "dark"):
383385
super().__init__(orientation=Gtk.Orientation.VERTICAL)
384386
# FUTURE: this is proposed layout for new API
385387
# self.device_label = Gtk.Label()
386-
# self.device_label.set_markup(device.name)
388+
# self.device_label.set_text(device.name)
387389
# self.device_label.get_style_context().add_class('device_name')
388390
# self.edit_icon = VariantIcon('edit', 'dark', 24)
389391
# self.detailed_description_label = Gtk.Label()
390392
# self.detailed_description_label.set_text(device.description)
391393
# self.backend_icon = VariantIcon(device.vm_icon, 'dark', 24)
392394
# self.backend_label = Gtk.Label(xalign=0)
393-
# self.backend_label.set_markup(str(device.backend_domain))
395+
# self.backend_label.set_text(str(device.backend_domain))
394396
#
395397
# self.title_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)
396398
# self.title_box.add(self.device_label)
@@ -405,7 +407,7 @@ def __init__(self, device: backend.Device, variant: str = "dark"):
405407
# self.add(self.attachment_box)
406408

407409
self.device_label = Gtk.Label()
408-
self.device_label.set_markup(device.name)
410+
self.device_label.set_text(device.name)
409411
self.device_label.get_style_context().add_class("device_name")
410412
self.device_label.set_xalign(Gtk.Align.CENTER)
411413
self.device_label.set_halign(Gtk.Align.CENTER)
@@ -447,7 +449,7 @@ def __init__(self, device: backend.Device, variant: str = "dark"):
447449

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

450-
label_markup = device.name
452+
label_markup = GLib.markup_escape_text(device.name)
451453
if (
452454
device.connection_timestamp
453455
and int(time.monotonic() - device.connection_timestamp) < 120

0 commit comments

Comments
 (0)