1817588 - NULL dereferences on MIME handler failures on OpenBSD

Status: RESOLVED FIXED

(Firefox :: Untriaged, defect)

Description

[Theo Buehler]

Steps to reproduce:

Try to download a magnet feed, selecting qbittorrent as handler.

Actual results:

The firefox main process crashed

Expected results:

Firefox should have downloaded the magnet feed and passed it to qbittorrent to handle.

Comment 1

[Theo Buehler]

Attachment: Bug 1817588 - Plug memory leak in g_app_info_launch_default_for_uri_openbsd()

content_type can be populated and result_uncertain can be true,
and in that case, content_type leaks.

Comment 2

[Theo Buehler]

Attachment: Bug 1817588 - Ensure openbsd uri handlers set GError on failure

There are various failure paths that result in NULL dereferences
of error. Fix this by setting the error explicitly to match the
API contract of the glib2 functions these replace. These crashes
are particularly annoying since they happen in the main process.

Depends on D170274

Comment 3

[Landry Breuil (:gaston)]

caused by

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1714919

Comment 5

[Landry Breuil (:gaston)]

Fwiw, i've backported the patch to thunderbird 110.0b4 which i'm testing, and i think g_warning is redundant with g_set_error since i have this on stderr:

[Parent 62186, Main Thread] WARNING: Could not find content type for URI: https://www.thunderbird.net/thunderbird/102.0/eoy/: 'glib warning', file /usr/obj/ports/thunderbird-110.0beta4/thunderbird-110.0/toolkit/xre/nsSigHandlers.cpp:167

** (thunderbird-default:62186): WARNING **: 22:35:03.536: Could not find content type for URI: https://www.thunderbird.net/thunderbird/102.0/eoy/
[Parent 62186, Main Thread] WARNING: Could not launch default application for URI: Could not find content type for URI: https://www.thunderbird.net/thunderbird/102.0/eoy/: 'glib warning', file /usr/obj/ports/thunderbird-110.0beta4/thunderbird-110.0/toolkit/xre/nsSigHandlers.cpp:167

** (thunderbird-default:62186): WARNING **: 22:35:03.536: Could not launch default application for URI: Could not find content type for URI: https://www.thunderbird.net/thunderbird/102.0/eoy/

since the initial intent of g_warning was to have some feedback on stderr, if setting error via g_set_error makes it printed to stderr, then we can delete the g_warning call.

without the patch, thunderbird crashed straight away at the same spot, so it definitely avoids that NULL deref.

:stransky, what do you think about it ?

Comment 6

[Theo Buehler]

Attachment: Bug 1817588 - Drop redundant g_warning() r=gaston,stransky

The errors set using g_set_error() are printed later, so these are
redundant. Drop them.

Depends on D170275

Comment 7

[Theo Buehler]

Attachment: Bug 1817588 - Use MakeScopeExit() instead of manual g_free() r=gaston,stransky

Depends on D170345

Comment 8

[Theo Buehler]

Attachment: Bug 1817588 - plug another memleak in OpenBSD helpers r=gaston,stransky

g_filename_from_uri() allocates the path it returns, so that
must also be freed.

Depends on D170402

Comment 9

[Theo Buehler]

Attachment: Bug 1817588 - Another leak: free app_info r=gaston,stransky

Depends on D170413

Comment 10

[Landry Breuil (:gaston)]

:stransky: can we have all the commits squashed in one (once reviewed) when landing ? phabricator can be super user-hostile for outsiders/newcomers...

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Theo, do you mind to create just one big patch for all those changes? It may be easier to review / land and also check all changes are in place.
Thanks.

Comment 12

[Theo Buehler]

Attachment: Bug 1817588 - Ensure OpenBSD URI handlers set GError on failure r=gaston,stransky

There are various failure paths that result in NULL dereferences
of error. Fix this by setting the error explicitly to match the
API contract of the glib2 functions these OpenBSD specific functions
replace. The crashes are particularly annoying since they happen in
the main process. In addition, plug a number of memory leaks.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/022f05513860
Ensure OpenBSD URI handlers set GError on failure r=gaston,stransky

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/022f05513860

Comment 15

[Rares Doghi, Desktop QA]

Unfortunately we dont have the correct environment to verify this issue, @Theo Buehler could please check on your end in our latest Nightly build and see if the issue is fixed or if it still occurs ? you can find the build here: https://nightly.mozilla.org/

Comment 16

[Landry Breuil (:gaston)]

(In reply to Rares Doghi from comment #15)

Unfortunately we dont have the correct environment to verify this issue, @Theo Buehler could please check on your end in our latest Nightly build and see if the issue is fixed or if it still occurs ? you can find the build here: https://nightly.mozilla.org/

there are no nightly builds for tier3 platforms, but it's been tested on our side and backported to the firefox 110 package shipped to OpenBSD users.