GNOME Bugzilla – Bug 733338
Don't segfault in GNetworkMonitor when IPv6 support is unavailable
Last modified: 2018-02-27 12:10:03 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
+ Trace 233840
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
Created attachment 281044 [details] [review] Patch2 (ignore IPv6 failure)
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.
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.
(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.)
same thing under windows xp 32bit, glib 2.42.0 g_network_monitor_get_default crashes if ipv6 isn't installed
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.
(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.
Second patch pushed to master, thanks.