New Security Issue
Original reporter: Daniel Holbert
Area: Platform component (libraries, tools)
Message
I work at Mozilla on Firefox's printing components, and we ran across a security-sensitive crash (a use-after-free) in Firefox, which turns out to be a bug in libgtk3, which is reproducible in Firefox as well as with a minimal c++ testcase.
It manifests as a crash in the function gtk_printer_get_name
, when running a program that calls "gtk_enumerate_printers" on systems that use a remote CUPS server. More details below.
This crash is actually already filed as #4672 (closed) , but when my colleague Emilio filed that issue, we weren't aware that it was a use-after-free. (Our crash reports were mistakenly reporting the access address as being a nullptr dereference, when in fact they were using a freed pointer.) It's of course your call whether it's better to morph that issue into a private bug, vs. track this in a new report, vs. something else.
We've found that an effective workaround in Firefox is to simply let gtk_enumerate_printers complete a full enumeration -- the crash only happens when the enumeration callback function returns true and stops the enumeration partway through. We're shipping that workaround (https://hg.mozilla.org/mozilla-central/rev/833e0bfa6d81 ) to mitigate the crash in Firefox, but I wanted to also report the bug upstream to you.
Here's a rough overview of what goes wrong, when the crash happens (based on my debugging/analysis when diagnosing this in Firefox):
- The client program calls
gtk_enumerate_printers
, passing in an enumeration callback function. -
gtk_enumerate_printers
callslist_printers_init
, which sets up a callback-handler for the named signal"printer-added"
, wiring that signal up to calllist_added_cb()
, which (when called) will call the client program's enumeration callback function. - Later on, we find ourselves in
cups_request_printer_list_cb()
to handle a response from the cups server with printer info. - In that
cups_request_printer_list_cb
function, we set up a local linked-list of printers,removed_printer_checklist
that we're comparing against and modifying as-we-go for some reason. This is the list that's going to get us into trouble!! - The main loop of
cups_request_printer_list_cb
iterates over the CUPS response data, generating GtkPrinter instances as it goes. - AND, during that loop: each time it creates a new
GtkPrinter
, it sends the"printer-added"
signal, which synchronously invokes thelist_added_cb
callback that I discussed at the start here. -
list_added_cb
calls the client program's own custom enumeration-callback-function. If that function returns true to stop the enumeration, then a bunch of data gets deleted (which is a problem since that data is still referenced in removed_printer_checklist)!! Specifically, that deletion happens as follows:list_added_cb
callsstop_enumeration
, which callslist_done_cb
, which callslist_printers_remove_backend
which destroys the print backend and frees the printer list. - Then we unwind back up to the
cups_request_printer_list_cb
loop (where we dispatched the "printer-added" signal from), and we keep looping over the cups response data; and when we get to the part where we try to search in ourremoved_printer_checklist
linked-list ofGtkPrinter*
pointers, we find that theGtkPrinter*
pointers in our linked list are all pointing to garbage data, and we potentially crash with a use-after-free when getting their name ingtk_printer_get_name
. Or, if we're lucky, we notice that the freed object's data doesn't look right for aGtkPrinter
, and we bail out from gtk_printer_get_name via theg_return_if_fail(GTK_IS_PRINTER(...))
early-return. - (That
g_return_if_fail
statement can be seen-to-be-failing via the environmental variableG_DEBUG=fatal-criticals
, so that this manifests as a crash even if we're lucky enough (as we usually are) for the deleted object to be detected as garbage by that check.)
I've got a sample minimal C++ program that triggers the bug, which I'm happy to provide the source for, though it appears this form doesn't let me include attachments. Please let me know how best to submit that; maybe as an attachment on the bug that results from this submission?
Thanks! ~Daniel Holbert