Skip to content

xapp-sn-watcher: Validate client-supplied icon pixmaps - #210

Open
zquestz wants to merge 1 commit into
linuxmint:masterfrom
zquestz:sn-watcher-validate-pixmaps
Open

xapp-sn-watcher: Validate client-supplied icon pixmaps#210
zquestz wants to merge 1 commit into
linuxmint:masterfrom
zquestz:sn-watcher-validate-pixmaps

Conversation

@zquestz

@zquestz zquestz commented Aug 29, 2026

Copy link
Copy Markdown

Any application on the session bus can crash xapp-sn-watcher (and with it
every tray icon on the panel) by exporting a StatusNotifierItem whose
IconPixmap property has the wrong type.

get_icon_surface() unpacks the property like this:

GVariantIter *iter;                        /* uninitialized */
g_variant_get (pixmaps, "a(iiay)", &iter); /* no-op on type mismatch */
if (iter == NULL)                          /* reads uninitialized stack */
    return NULL;

When a client sends any signature other than a(iiay), g_variant_get()
logs a critical and returns without touching iter, so the NULL check reads
an uninitialized stack slot and g_variant_iter_loop() dereferences garbage.
The same applies to AttentionIconPixmap and OverlayIconPixmap, which go
through the same function. The ToolTip handler already checks its type before
unpacking; the pixmap path does not.

This PR:

  • checks the variant type before unpacking, and logs a warning naming the
    item and the offending type instead of crashing
  • initializes the iterator so any future mismatch fails safe
  • treats the pixmap geometry as untrusted: dimensions are capped at 4096
    (far above any real tray icon, far below anything that can overflow), and
    the size arithmetic is done in gsize, so width * height * 4 can neither
    overflow a gint nor be truncated by g_memdup()'s guint length argument
  • uses g_memdup2() on GLib >= 2.68, with the same version guard already
    used in xapp-gpu-offload-helper.c

Reproduced against master by registering an SNI client whose IconPixmap is
declared and returned as ay: the stock watcher segfaults as soon as it
fetches properties, the patched watcher logs

Ignoring icon pixmap for ':1.10': expected type 'a(iiay)' but got 'ay'

and keeps running. Verified in a clean Ubuntu 24.04 environment on a private
session bus.

Found while investigating the tray instability Cinnamon users hit when
Signal and Slack shipped Electron 43.3.x/43.4.0 (electron/electron#52674,
signalapp/Signal-Desktop#7992). The Electron regression is fixed upstream in
43.4.1, but it demonstrated that one misbehaving client can take down the
whole tray; this closes the crash half of that. A follow-up PR addresses a
separate defect in how exiting clients' icons are removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant