After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 733338 - Don't segfault in GNetworkMonitor when IPv6 support is unavailable
Don't segfault in GNetworkMonitor when IPv6 support is unavailable
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: network
2.40.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-07-17 19:59 UTC by Erik van Pienbroek
Modified: 2018-02-27 12:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch1 (input validation in g_inet_address_mask_equal (842 bytes, patch)
2014-07-17 19:59 UTC, Erik van Pienbroek
committed Details | Review
Patch2 (ignore IPv6 failure) (1.12 KB, patch)
2014-07-17 20:00 UTC, Erik van Pienbroek
committed Details | Review

Description Erik van Pienbroek 2014-07-17 19:59:24 UTC
Created attachment 281043 [details] [review]
Patch1 (input validation in g_inet_address_mask_equal

An user of the Fedora mingw-glib2 2.40.0 package has reported a segfault when using the GNetworkMonitor:

Program received signal SIGSEGV, Segmentation fault.
0x6d5aa3ff in g_inet_address_mask_equal (mask=0x58e8db8,
    mask2=mask2@entry=0x0) at ginetaddressmask.c:468
468     ginetaddressmask.c: No such file or directory.
(gdb) bt
  • #0 g_inet_address_mask_equal
    at ginetaddressmask.c line 468
  • #1 g_network_monitor_base_add_network
    at gnetworkmonitorbase.c line 415
  • #2 g_network_monitor_base_constructed
    at gnetworkmonitorbase.c line 102
  • #3 g_object_new_internal
    at gobject.c line 1814
  • #4 g_object_new_valist
    at gobject.c line 2044
  • #5 g_initable_new_valist
    at ginitable.c line 224
  • #6 g_initable_new
    at ginitable.c line 146
  • #7 try_implementation
    at giomodule.c line 755
  • #8 _g_io_module_get_default
    at giomodule.c line 857
  • #9 g_network_monitor_get_default
    at gnetworkmonitor.c line 74
  • #10 webkit_web_view_load_error_cb
    at /home/pawel/src/midori/trunk/midori/midori-view.c line 1222
  • #11 webkit_marshal_BOOLEAN__OBJECT_STRING_BOXED
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #12 g_closure_invoke
    at gclosure.c line 768
  • #13 signal_emit_unlocked_R
    at gsignal.c line 3623
  • #14 g_signal_emit_valist
    at gsignal.c line 3319
  • #15 g_signal_emit_by_name
  • #16 WebKit::FrameLoaderClient::dispatchDidFailLoad
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #17 WebCore::FrameLoader::checkLoadCompleteForThisFrame
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #18 WebCore::FrameLoader::checkLoadComplete
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #19 WebCore::FrameLoader::receivedMainResourceError
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #20 WebCore::CachedResource::checkNotify
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #21 WebCore::CachedResource::error
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #22 WebCore::SubresourceLoader::didFail
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #23 WebCore::ResourceLoader::didFail
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #24 sendRequestCallback
    from F:\mingw\bin\libwebkitgtk-3.0-0.dll
  • #25 g_task_return_now
    at gtask.c line 1077
  • #26 g_task_return
    at gtask.c line 1130
  • #27 ??
    from F:\mingw\bin\libsoup-2.4-1.dll
  • #28 ??
  • #29 ??
  • #30 ??
  • #31 ??
  • #32 ??
  • #33 ??

The segfault is caused by the fact that g_inet_address_mask_equal doesn't perform any input validation and tries to dereference a NULL pointer (on the mask2 variable). Attached patch adds input validation to g_inet_address_mask_equal, so that a warning is triggered instead of a segfault.

The root cause (which causes the mask2 variable to be empty) is in g_network_monitor_base_constructed. It tries to create a GInetAddressMask for the address '::/0', but on some environments like Windows with IPv6 support disabled this can fail. If this is the case then it won't be needed to set up a network monitor for it.

These two patches are confirmed to resolve the mentioned segfault
Comment 1 Erik van Pienbroek 2014-07-17 20:00:42 UTC
Created attachment 281044 [details] [review]
Patch2 (ignore IPv6 failure)
Comment 2 Dan Winship 2014-10-26 15:45:41 UTC
Comment on attachment 281044 [details] [review]
Patch2 (ignore IPv6 failure)

>       mask = g_inet_address_mask_new_from_string ("::/0", NULL);
>-      g_network_monitor_base_add_network (monitor, mask);
>-      g_object_unref (mask);
>+      if (mask)
>+        {
>+          /* On some environments (for example Windows without IPv6 support
>+           * enabled) the string "::/0" can't be processed and causes
>+           * g_inet_address_mask_new_from_string to return NULL */

I would say that that's a bug... gio should always be able to parse IPv6 addresses, even if it won't be able to connect to them.
Comment 3 Erik van Pienbroek 2014-10-26 20:39:51 UTC
According to https://developer.gnome.org/gio/stable/gio-GInetAddressMask.html#g-inet-address-mask-new-from-string the g_inet_address_mask_new_from_string function can return NULL if the given string could not be parsed..so it is questionable whether this really is a gio bug. I don't think it should be glib/gio's job to parse IPv6 addresses by itself if the native host OS networking API (in this case Windows) isn't able to do so.
Comment 4 Dan Winship 2014-10-27 15:38:22 UTC
(In reply to comment #3)
> According to
> https://developer.gnome.org/gio/stable/gio-GInetAddressMask.html#g-inet-address-mask-new-from-string
> the g_inet_address_mask_new_from_string function can return NULL if the given
> string could not be parsed

eg, if you pass "sadfadsfsagasdfsaf"

> ..so it is questionable whether this really is a gio
> bug. I don't think it should be glib/gio's job to parse IPv6 addresses by
> itself if the native host OS networking API (in this case Windows) isn't able
> to do so.

It is not GLib's job to create an IPv6 connection if the OS can't, but creating a connection can fail for lots of other reasons anyway, and so programs will be checking for that, and handling it appropriately. It is annoying for programs to have to deal with the possibility that they can't even *parse* a known-to-be-correct IPv6 address (especially since 99% of the time, they will be able to, and so no one is ever actually going to write the code to deal with it failing, and they'll just crash instead.)
Comment 5 Andrea Zagli 2014-11-08 09:29:15 UTC
same thing under windows xp 32bit, glib 2.42.0

g_network_monitor_get_default crashes if ipv6 isn't installed
Comment 6 Philip Withnall 2018-02-27 12:05:22 UTC
Review of attachment 281044 [details] [review]:

(In reply to Dan Winship from comment #4)
> It is not GLib's job to create an IPv6 connection if the OS can't, but
> creating a connection can fail for lots of other reasons anyway, and so
> programs will be checking for that, and handling it appropriately. It is
> annoying for programs to have to deal with the possibility that they can't
> even *parse* a known-to-be-correct IPv6 address (especially since 99% of the
> time, they will be able to, and so no one is ever actually going to write
> the code to deal with it failing, and they'll just crash instead.)

Sure, but in this case, the inet mask being parsed is just for the purposes of matching the default route and determining reachability — both of which aren’t going to happen for IPv6 if the kernel doesn’t support it. So we might as well just push this patch, I think.

The latest versions of GLib require Windows 7 (https://wiki.gnome.org/Projects/GLib/SupportedPlatforms), which apparently makes IPv6 support mandatory (https://support.microsoft.com/en-gb/help/929852/how-to-disable-ipv6-or-its-components-in-windows), so the need for us to support parsing IPv6 addresses/masks because the OS doesn’t support them has gone away.
Comment 7 Philip Withnall 2018-02-27 12:06:49 UTC
(In reply to Philip Withnall from comment #6)
> The latest versions of GLib require Windows 7
> (https://wiki.gnome.org/Projects/GLib/SupportedPlatforms), which apparently
> makes IPv6 support mandatory

…which might make this patch obsolete, but I think for the purposes of robustness, we might as well push it.
Comment 8 Philip Withnall 2018-02-27 12:09:57 UTC
Second patch pushed to master, thanks.