Use-after-free when `replaces_id` and a synchronous tag name the same notification

[cschramm]

Summary

If a Notify call carries both a replaces_id pointing at a still-live notification and an x-canonical-private-synchronous / x-dunst-stack-tag hint, mako frees the notification it is currently populating and then keeps using it. The result is a use-after-free that reliably aborts the daemon inside apply_each_criteria().

This is not limited to exotic configs — it reproduces with the stock configuration, because the global criteria always specifies button_bindings.left, whose action_name is a heap string.

Environment

• mako 1.11.0 (Arch Linux mako 1.11.0-1)
• glibc 2.44, niri / wlroots, libnotify notify-send
• Also present in master — dbus/xdg.c:362-371 is unchanged, and the code has been this way since the tag feature landed in 4a30dfb (v1.6).

Steps to reproduce

With the default config (no ~/.config/mako/config needed):

ID=$(notify-send --app-name=test --expire-time=30000 --print-id \
       --hint=string:x-canonical-private-synchronous:mytag "a" "first")

notify-send --app-name=test --expire-time=30000 \
       --hint=string:x-canonical-private-synchronous:mytag \
       --replace-id=$ID "a" "second"

The second call must arrive while the first notification is still on screen.

Expected: the notification is replaced.

Actual: mako aborts.

Failed to show notification: GDBus.Error:org.freedesktop.DBus.Error.NoReply: Remote peer disconnected

mako[485361]: free(): chunks in smallbin corrupted
systemd-coredump: Process 485361 (mako) of user 1000 dumped core.

The exact glibc diagnostic varies with heap state; I have captured both
free(): double free detected in tcache 2 and free(): chunks in smallbin corrupted
from the same code path.

Root cause

In handle_notify() (dbus/xdg.c), a replaces_id naming a live notification causes mako to reuse that notification object, which is still linked into state->notifications:

    if (replaces_id > 0) {
        notif = get_notification(state, replaces_id);   /* xdg.c:107 */
    }
    if (notif) {
        reset_notification(notif);                      /* xdg.c:112 — style is NOT reset */
    }

Further down, once notif->tag has been parsed from the hints, the tag-replacement block searches the same list:

    if (notif->tag) {
        struct mako_notification *replace_notif =
            get_tagged_notification(state, notif->tag, app_name);   /* xdg.c:364 */
        if (replace_notif) {
            notif->id = replace_notif->id;
            wl_list_insert(&replace_notif->link, &notif->link);
            destroy_notification(replace_notif);                    /* xdg.c:368 */
            replaces_id = notif->id;
        }
    }

Because notif is itself in state->notifications and carries that exact tag and app name, get_tagged_notification() returns notif itself. So:

1. wl_list_insert(&notif->link, &notif->link) self-links the list entry.
2. destroy_notification(notif) runs finish_style() — freeing button_bindings.*.command and .action_name — and then free(notif).
3. Execution continues on the freed notif into apply_each_criteria() (xdg.c:385).
4. apply_style() → copy_binding() → finish_binding() frees those binding strings a second time.

finish_binding() frees without clearing, which is fine on its own but leaves nothing to catch the reuse:

static void finish_binding(struct mako_binding *binding) {
    free(binding->command);        /* config.c:147 */
    free(binding->action_name);    /* config.c:148 */
}

Backtrace (stock config)

#10 __GI___libc_free (mem=<optimized out>) at malloc.c:3184
#11 finish_binding (binding=0x556004e8f520) at config.c:148
#12 copy_binding (dst=0x556004e8f520, src=0x556004e838f0) at config.c:165
#13 apply_style (target=<optimized out>, style=0x556004e83800) at config.c:381
#14 apply_each_criteria (criteria_list=0x7ffdc3a060e0, notif=0x556004e8f410) at criteria.c:435
#15 handle_notify (msg=0x5560051797a0, ...) at dbus/xdg.c:385
#16 method_callbacks_run (...) at sd-bus/bus-objects.c:522

This one is worth highlighting — it is not a harmless double free of a dead pointer. The stale chunk had already been recycled into another live allocation by the time it was freed again:

(gdb) p *dst
$1 = {action = MAKO_BINDING_INVOKE_ACTION, command = 0x0,
      action_name = 0x5560051eb3e0 "monospace 10"}     <-- freed "default" chunk, now the font string
(gdb) p *src
$2 = {action = MAKO_BINDING_INVOKE_ACTION, command = 0x0,
      action_name = 0x556004e83ad0 "default"}

dst->action_name was strdup("default") from init_default_style(); after the premature destroy_notification() the chunk was reallocated for the font string "monospace 10", which copy_binding() then frees while it is still in use.

With a config that sets an exec binding, the same crash lands one line earlier on free(binding->command) (config.c:147), and the dead notif's app_name / summary / tag all read back as mangled tcache pointers.

Suggested fix

A notification cannot meaningfully replace itself — when replace_notif == notif, the desired end state already holds: the id is correct and the list position is unchanged. Skipping the block is sufficient:

 	if (notif->tag) {
 		// Find and replace the existing notfication with a matching tag
 		struct mako_notification *replace_notif = get_tagged_notification(state, notif->tag, app_name);
-		if (replace_notif) {
+		if (replace_notif && replace_notif != notif) {
 			notif->id = replace_notif->id;
 			wl_list_insert(&replace_notif->link, &notif->link);
 			destroy_notification(replace_notif);
 			replaces_id = notif->id;
 		}
 	}

replaces_id is left equal to notif->id in that case, so the if (replaces_id != notif->id) guard at xdg.c:379 correctly continues to skip insert_notification().

Optionally, having finish_binding() null the pointers after freeing would make this class of bug fail loudly at the second use rather than corrupting the heap:

static void finish_binding(struct mako_binding *binding) {
    free(binding->command);
    free(binding->action_name);
    binding->command = NULL;
    binding->action_name = NULL;
}

Verification of the fix

Built 1.11.0 with only the guard above applied and re-ran, against the same config:

Case	Result
--replace-id + matching tag (the repro)	no crash, id preserved
5 rapid rounds, both mechanisms	no crash, id stable, no stacking
tag-only replacement	still replaces, id stable
--replace-id only, no tag	still replaces, id stable

Reachability

For completeness, since a use-after-free invites the question — I could not find a path that crosses a privilege boundary. Triggering it requires a caller that sets a synchronous tag hint and a replaces_id naming a live notification, and the obvious candidates for a less-privileged caller do not:

• org.freedesktop.portal.Notification — does forward replaces_id, but constructs a fixed hint set (desktop-entry, urgency) and does not pass arbitrary hints through, so a sandboxed app cannot inject the tag. Confirmed on the wire and by three back-to-back AddNotification calls with an identical id: no crash. (notif->tag is "" there, which get_tagged_notification() already skips via its strlen() != 0 check.)
• Firefox 153 — reaches the daemon through libnotify directly, and neither libxul.so nor libnotify.so.4 contains either hint name, so a web page's JS notification tag is not mapped onto the fdo synchronous hints.

So in practice this needs a cooperating same-user client, which makes it a robustness bug rather than a security issue — filing it accordingly rather than through a security contact. It is still a real use-after-free that frees a live allocation, and the affected strings are attacker-influenced in length and content, so it seemed worth writing up carefully.

Notes

Clients that remember a notification id and set a stable per-application tag hit this whenever an update arrives before the previous popup expires — a natural pattern for progress/status notifications. Sending only the tag is a working client-side workaround, since mako already carries the old id across a tag replacement.