Bugs item #2868910, was opened at 2009-09-28 15:07
Message generated for change (Comment added) made by sf-robot
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2868910&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: SSL/TLS
Group: crash
>Status: Closed
Resolution: Works For Me
Priority: 5
Private: No
Submitted By: Tomek Bury (tomekbury)
Assigned to: Daniel Stenberg (bagder)
Summary: dangling pointer on reused ssl connections

Initial Comment:
When libCurl reuses existing SSL/TLS connection it doesn't call ssl ctx function set by CURLOPT_SSL_CTX_FUNCTION and CURLOPT_SSL_CTX_DATA. Instead it blindly reuses ssl context modified by whichever easy request was using it previously.

As result, example code using ssl ctx callback to call SSL_CTX_set_cert_verify_callback (as per curlx.c example) may lead to use of dangling pointer.

Scenario:

1. easy reqest 1 sets CURLOPT_SSL_CTX_FUNCTION and CURLOPT_SSL_CTX_DATA, data points to request's data.
2. easy_perform called on reqest 1 triggers ssl ctx function, which in turn calls SSL_CTX_set_cert_verify_callback
3. later on verify callback fires and uses request's data.
4. request 1 completes, request's data is deleted

so far so good...

5. easy reqest 1 sets CURLOPT_SSL_CTX_FUNCTION and CURLOPT_SSL_CTX_DATA, data points to 2nd request's data.
6. easy_perform called on request 2 does NOT triggers ssl ctx function
7. later on verify callback fires and uses the 1st request's data but now it's a dangling pointer- .
8. memory corruption

----------------------------------------------------------------------

>Comment By: SourceForge Robot (sf-robot)
Date: 2009-12-12 02:20

Message:
This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-11-27 15:19

Message:
This issue needs to be taken to the curl-library mailing list to be
discussed as it's not exactly a bug, but more a feature that possibly isn't
done the best way when implemented as it currently is.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-10-30 22:47

Message:
The example can of course be improved, please post your patches.

For the CURLOPT_SSL_CTX_FUNCTION problem I would also appreciate patches,
but I would prefer to see this issue discussed on the curl-library mailing
list first properly since it isn't a plain bug fix but it will change the
behavior of existing functionality and I want that to be brought to other
people's attention before we even consider changing it.

----------------------------------------------------------------------

Comment By: Tomek Bury (tomekbury)
Date: 2009-10-23 13:08

Message:
The dangling pointer is in the app, not in the lib. The lib simply creates
an environment and provides documentation and usage example that creates
dangling pointer situation. The example code is in dosc/examples/curlx.c.

1. sslctxparm p is a request specific data
2. register callback and pass pointer to that data
   res = curl_easy_setopt(p.curl, CURLOPT_SSL_CTX_FUNCTION, sslctxfun) ;
   curl_easy_setopt(p.curl, CURLOPT_SSL_CTX_DATA, &p);
3. in curl callback register ssl callback and pass the same data
  SSL_CTX_set_cert_verify_callback(ctx, ssl_app_verify_callback, parm);
4. in ssl callback dereference the pointer
static int ssl_app_verify_callback(X509_STORE_CTX *ctx, void *arg)
{
  sslctxparm * p = (sslctxparm *) arg;
  int ok;

  if (p->verbose > 2)
    BIO_printf(p->errorbio,"entering ssl_app_verify_callback\n");

In the example, luckily the pointer points to a single static structure
for one and only request sent. In the real life app with more requests the
request-specific structures will be created and destroyed, and perhaps,
silently reused after being destroyed. I'm not too concerned about the
dangling pointer itself, because this can be avoided by careful coding.

There are 2 severe problems though.

First, it's a non trivial task to figure out what's actually happening.
There's an example how (not) to use the SSL and there's nothing in the spec
to warn about that behaviour.

Second, and more important, only some of the easy hanles will get the
chance to access SSL context, while other will silently inherit modified
context without a chance to undo the existsing changes because
CURLOPT_SSL_CTX_FUNCTION will not be called.

IMO libCURL should reset the existsing SSL connection settings to default
state before reusing it (curl_easy_init, curl_easy_reset) and always call
SSL callback, so that every easy handle can get access to SSL context, not
just the lucky ones.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-10-21 14:53

Message:
Can you please provide an example source code that shows a memory
corruption case that isn't the fault of the app?

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-10-01 08:48

Message:
I don't see how that memory corruption is really libcurl's fault, since it
doesn't call the callback the app clearly knows this and should act
accordingly.

But the fact that it doesn't call the ssl context callback on a re-used
connection might be something to adress. I've personally not that into
exactly what that context/callback is typically used for so I don't know
how such a change might impact existing users. It does require tweaking the
code somewhat though to make it act this way.

----------------------------------------------------------------------

Comment By: Tomek Bury (tomekbury)
Date: 2009-09-28 16:52

Message:
I made a typo:

5. easy request 1 - should be request 2, not 1.

The whole point is that changes made to ssl context by one easy handle are
silently reused by different, unrelated easy handle.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2868910&group_id=976
Received on 2009-12-12