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 562191 - Segfault in soup_cookie_applies_to_uri when uri->path is NULL
Segfault in soup_cookie_applies_to_uri when uri->path is NULL
Status: RESOLVED DUPLICATE of bug 528882
Product: libsoup
Classification: Core
Component: Misc
2.24.x
Other All
: Normal major
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on:
Blocks:
 
 
Reported: 2008-11-24 23:00 UTC by Mark Lee
Modified: 2008-12-09 19:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
alternate patch (370 bytes, patch)
2008-11-25 02:03 UTC, Dan Winship
none Details | Review

Description Mark Lee 2008-11-24 23:00:21 UTC
Steps to reproduce:
Using libsoup 2.24.2:

SoupCookie *cookie has all non-NULL fields, SoupURI *uri has a non-NULL scheme, host, and port attributes (and a NULL path attribute). soup_cookie_applies_to_uri() segfaults when trying to access uri->path[plen], where plen == 0.

Stack trace:
  • #0 soup_cookie_applies_to_uri
    at soup-cookie.c line 885
  • #1 soup_cookie_jar_get_cookies
    at soup-cookie-jar.c line 168
  • #2 request_started
    at soup-cookie-jar.c line 301
  • #3 request_started
    at soup-session-feature.c line 83
  • #4 soup_marshal_VOID__OBJECT_OBJECT
    at soup-marshal.c line 90
  • #5 IA__g_closure_invoke
    at gclosure.c line 490

Other information:
The crash disappears if I change line 885:

-	if (uri->path[plen] && uri->path[plen] != '/')
+	if (uri->path && uri->path[plen] && uri->path[plen] != '/')

Although, I'm not sure if this invalidates the algorithm.
Comment 1 Dan Winship 2008-11-24 23:30:41 UTC
(In reply to comment #0)
> SoupCookie *cookie has all non-NULL fields, SoupURI *uri has a non-NULL scheme,
> host, and port attributes (and a NULL path attribute).

SoupURI->path is defined to always be non-%NULL, and soup_uri_new() enforces this (unless you do soup_uri_new(NULL) to create an empty URL, but in that case you're responsible for setting it up correctly yourself, and the docs state that path must always be non-NULL).

What program is this crash happening in? How are you setting the message's URI?
Comment 2 Mark Lee 2008-11-25 00:01:21 UTC
(In reply to comment #1)
> What program is this crash happening in? How are you setting the message's URI?

It's a Vala-based screenscraping app that I'm working on.  Looking at the SoupLogger output, I think the problem stems from the fact that I'm POSTing to an HTTPS URI via my local proxy.

Here's the (sanitized) Vala code (the proxy URI and cookie were set earlier):

Message new_msg = new Message ("POST", "https://ssl.example.com/form.php");
string encoded_data = "submit=Submit";
new_msg.set_request ("application/x-www-form-urlencoded",
                     Soup.MemoryUse.COPY, encoded_data,
                     encoded_data.length);
session.queue_message (new_msg, form_cb);

The crash occurs after the message is queued.

Here's the (sanitized) SoupLogger output, after applying my workaround:

> CONNECT ssl.example.com:443 HTTP/1.1
> Soup-Debug-Timestamp: 1227566604
> Soup-Debug: SoupSessionAsync 1 (0x8058010), SoupMessage 2 (0x805b8b0), SoupSocket 2 (0x8059938)
> Host: ssl.example.com
> Cookie: foo=bar

< HTTP/1.0 200 Connection established
< Soup-Debug-Timestamp: 1227566609
< Soup-Debug: SoupMessage 2 (0x805b8b0)
< Proxy-Agent: Privoxy/3.0.6

> POST /form.php HTTP/1.1
> Soup-Debug-Timestamp: 1227566609
> Soup-Debug: SoupSessionAsync 1 (0x8058010), SoupMessage 3 (0x805b858), SoupSocket 2 (0x8059938)
> Host: ssl.example.com
> Content-Type: application/x-www-form-urlencoded
> Cookie: foo=bar
>
> submit=Submit
Comment 3 Dan Winship 2008-11-25 01:51:04 UTC
(In reply to comment #2)
> It's a Vala-based screenscraping app that I'm working on.  Looking at the
> SoupLogger output, I think the problem stems from the fact that I'm POSTing to
> an HTTPS URI via my local proxy.

Ouch. Yes, if you have a cookie for your proxy server, it will crash.
Comment 4 Dan Winship 2008-11-25 02:03:13 UTC
Created attachment 123355 [details] [review]
alternate patch

can you verify that this patch also fixes the bug (even without your patch to soup-cookie.c)?
Comment 5 Dan Winship 2008-11-25 02:06:08 UTC
(In reply to comment #2)
> Message new_msg = new Message ("POST", "https://ssl.example.com/form.php");
> string encoded_data = "submit=Submit";
> new_msg.set_request ("application/x-www-form-urlencoded",
>                      Soup.MemoryUse.COPY, encoded_data,
>                      encoded_data.length);

btw, this would be easier with soup_form_request_new() if that's wrapped by vala. Eg, in C it would be just:

    new_msg = soup_form_request_new ("POST", "https://ssl.example.com/form.php",
                                     "submit", "Submit",
                                     NULL);
Comment 6 Mark Lee 2008-11-25 02:21:41 UTC
(In reply to comment #4)
> Created an attachment (id=123355) [edit]
> alternate patch
> 
> can you verify that this patch also fixes the bug (even without your patch to
> soup-cookie.c)?
> 

This patch (on a pristine tarball of 2.24.2) also fixes the segfault.
Comment 7 Dan Winship 2008-11-25 14:25:51 UTC
This is now fixed on gnome-2-24 branch and I've put out a new 2.24.2.1 tarball with the fix.

I'm leaving the bug open because it's not yet fixed on trunk, and the fix will be slightly different there, and may tie in to the re-fix for bug 528882, and also while looking at your original SoupCookie patch, I noticed another really dumb bug in soup_cookie_applies_to_uri() that I should fix too. (It doesn't check that plen < strlen(uri->path), meaning it may read memory beyond the end of uri->path. Since it doesn't *write* there, it's probably not going to cause any crashes, but it's still broken.)
Comment 8 Dan Winship 2008-12-09 19:55:13 UTC
fixed in trunk. dupping to 528882 to remind me of the issues here when i fix that


*** This bug has been marked as a duplicate of 528882 ***