1282419 - Large libnotify alert images trigger a SIGTERM

Status: RESOLVED FIXED

(Toolkit Graveyard :: Notifications and Alerts, defect, P1)

Description

[Lina Butler [:lina]]

STR:

1. Open https://tests.peter.sh/notification-generator/.
2. Select "Huge icon (PNG, 3333x5230)" from the "Icon" drop-down menu.
3. Click "Display the notification."

The GDB backtrace shows a call to `notify_notification_show`, so we at least make it that far.

Comment 1

[rugk]

I can still reproduce it on GNOME version 3.24.2 with Firefox 55.

Test site: https://files.hboeck.de/crashff.html

Comment 3

[Matthew N. [:MattN]]

See some more info in bug 1395940.

Comment 4

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I see that we're always decoding the image to its native size [1].

This seems to be getting stuck in dbus-daemon: when we send a large enough image it rejects it as out of spec (see [2] and [3]), then Firefox halts on the broken connection [4], because glib dbus connections default to exit on close. The notification example is 66.5 MB of RGBA, DBUS_MAXIMUM_ARRAY_LENGTH is 64 MB.

We should probably resize the image if it is going to go over the limit.

It may be a bug for glib to be sending arrays this large as the dbus documentation is pretty clear about the size limit [5].

(At least this what I found through testing on Lubuntu 17.10, I didn't see notify_notification_show explicitly in the backtrace so I may be looking at a different issue?)

[1] https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/toolkit/components/alerts/AlertNotification.cpp#250-254
[2] https://cgit.freedesktop.org/dbus/dbus/tree/dbus/dbus-marshal-validate.c?id=2f8f4d619b16b134671521c2b4aea3a94fb47848#n447
[3] https://cgit.freedesktop.org/dbus/dbus/tree/dbus/dbus-transport.c?id=2f8f4d619b16b134671521c2b4aea3a94fb47848#n1186
[4] https://github.com/GNOME/glib/blob/glib-2-54/gio/gdbusconnection.c#L802-L807
[5] https://dbus.freedesktop.org/doc/api/html/group__DBusProtocol.html#ga5265afa08a4c8d9f31b287a57e8cb217

Comment 5

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Don't attempt to notify with a large icon

Resizing turned out to be more of an ordeal than I expected. In the interest of getting this fixed now, here's a patch that prevents us from sending such large images.

The size limit isn't exactly 64MB because the image data array is wrapped in the hints array, which has the same limit. I added a bit of fudge beyond the 60 bytes that are definitely needed by libnotify. Fortunately we don't set any other hints so there aren't known variable sizes to take into consideration, but parts of the message depend on the protocol.

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 8932682 [details] [diff] [review]
Don't attempt to notify with a large icon

Thank you for the analysis.

If not pushing future patches to reviewboard, then please update ~/.hgrc to
include more context in patches by default.  This might be the relevant
portion:

[diff]
git = true
showfunc = 1
unified = 8

Comment 7

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Thanks karlt, and sorry about the ugly patch, I'll make those changes and try to remember reviewboard next time.

Try looks ok.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=52016dfa7941711286bbc7c9fbdb5a2b90452fa6

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2007a896005
Don't send a large notify image. r=karlt

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/a2007a896005