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 727890 - soup_content_sniffer_real_sniff segfault
soup_content_sniffer_real_sniff segfault
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other OpenBSD
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-04-09 12:44 UTC by Antoine Jacoutot
Modified: 2014-04-10 14:11 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
g_str_has_prefix: don't call strlen(str) (1.29 KB, patch)
2014-04-09 14:01 UTC, Dan Winship 		committed Details | Review

Description Antoine Jacoutot 2014-04-09 12:44:19 UTC 
Hi.

I am seeing a reproducible segfault in soup_content_sniffer_real_sniff when using webkit (in this example, MiniBrowser):

(gdb) bt

+ Trace 233452

  • #0 strlen
    at /usr/src/lib/libc/string/strlen.c line 43
  • #1 g_str_has_prefix
    at gstrfuncs.c line 2803
  • #2 soup_content_sniffer_real_sniff
    from /usr/local/lib/libsoup-2.4.so.8.0
  • #3 read_and_sniff
    from /usr/local/lib/libsoup-2.4.so.8.0
  • #4 soup_content_sniffer_stream_is_ready
    from /usr/local/lib/libsoup-2.4.so.8.0
  • #5 io_read
    from /usr/local/lib/libsoup-2.4.so.8.0
  • #6 io_run_until
    from /usr/local/lib/libsoup-2.4.so.8.0
  • #7 try_run_until_read
    from /usr/local/lib/libsoup-2.4.so.8.0
  • #8 read_ready_cb
    from /usr/local/lib/libsoup-2.4.so.8.0
  • #9 g_main_dispatch
    at gmain.c line 3064
  • #10 g_main_context_dispatch
    at gmain.c line 3663
  • #11 g_main_context_iterate
    at gmain.c line 3734
  • #12 g_main_loop_run
    at gmain.c line 3928
  • #13 NetworkProcessMain
    from /usr/local/lib/libwebkit2gtk-3.0.so.1.0
  • #14 _start
    from /usr/local/libexec/WebKitNetworkProcess
  • #15 ?? 






Comment 1


Dan Winship





          2014-04-09 14:01:48 UTC
        



I'm going to try to blame this on GLib...






Comment 2


Dan Winship





          2014-04-09 14:01:58 UTC
        



Created attachment 273895 [details] [review]
g_str_has_prefix: don't call strlen(str)

There's no reason to check the length of @str in g_str_has_prefix(),
since if it's shorter than @prefix, the strncmp() will fail anyway.
And besides making the function less efficient, it also breaks code
like:

    if (buf->len >=3 && g_str_has_prefix (buf->data, "foo"))
      ...

which really looks like it ought to work whether buf->data is
nul-terminated or not.






Comment 3


Antoine Jacoutot





          2014-04-09 14:26:46 UTC
        



(In reply to comment #2)
> Created an attachment (id=273895) [details] [review]
> g_str_has_prefix: don't call strlen(str)

Hi Dan.

This seems to do the trick :-)
I haven't been unable to trigger the segfault so far..






Comment 4


Allan Streib





          2014-04-10 03:24:56 UTC
        



Found same issue, see

https://github.com/conformal/xombrero/issues/58

And you're right, there's no need to check the length of str at all, my proposed fix to g_str_has_prefix used strnlen limited by the length of the prefix. But even that is not necessary.






Comment 5


Allison Karlitskaya (desrt)





          2014-04-10 14:08:31 UTC
        



Review of attachment 273895 [details] [review]:

You can commit this now, if you like, or you can try to do it open-coded to avoid taking the strlen() of the prefix when very likely we will reject on the very first character comparison...

::: glib/gstrfuncs.c
@@ +2798,3 @@
   g_return_val_if_fail (prefix != NULL, FALSE);
 
+  return strncmp (str, prefix, strlen (prefix)) == 0;

This is obviously better than the existing code.






Comment 6


Dan Winship





          2014-04-10 14:11:02 UTC
        



pushed to master and glib-2-40