Bug #50557 for Crypt-SSLeay: Crypt-SSLeay with openssl 1.0 has memory corruption issue

Fri Oct 16 07:12:15 2009 mmaslano [...] redhat.com - Ticket created

Subject:

Crypt-SSLeay with openssl 1.0 has memory corruption issue

perl script that uses Crypt::SSLeay is crashing since upgrade to openssl 1.0. Backtrace head with various debuginfo installed: #0 freelist_insert (ctx=0xa18c140, for_read=1, sz=34120, mem=0xa0d6a80) at s3_both.c:645 #1 0x0072f305 in ssl3_release_read_buffer (s=0xa0c4780) at s3_both.c:762 #2 0x0072b60c in ssl3_free (s=0xa0c4780) at s3_lib.c:2151 #3 0x00733d75 in tls1_free (s=0xa0c4780) at t1_lib.c:163 #4 0x00742041 in SSL_free (s=0xa0c4780) at ssl_lib.c:581 #5 0x00a6a7c5 in XS_Crypt__SSLeay__Conn_free (my_perl=0x9242008, cv=0xa002d54) at SSLeay.c:521 It looks like the problem is with free'ing. SSL_CTX_free() is called as first and then SSL_free(). IMHO these calls should be switched and SSL_free should be called as first. I'm not familiar with XS module, therefore I'd rather wait for your opinion. Whole bug report could be found here: https://bugzilla.redhat.com/show_bug.cgi?id=521342

Subject:

test.pl

Fri Oct 16 17:53:56 2009 david [...] landgren.net - Correspondence added

Subject:	Re: [rt.cpan.org #50557] Crypt-SSLeay with openssl 1.0 has memory corruption issue
Date:	Fri, 16 Oct 2009 23:53:49 +0200
To:	bug-Crypt-SSLeay [...] rt.cpan.org
From:	David Landgren <david [...] landgren.net>

Marcela Maslanova via RT wrote, some time around 16/10/2009 13:12: Show quoted text

> Fri Oct 16 07:12:15 2009: Request 50557 was acted upon. > Transaction: Ticket created by mmaslano@redhat.com > Queue: Crypt-SSLeay > Subject: Crypt-SSLeay with openssl 1.0 has memory corruption issue > Broken in: 0.57 > Severity: Important > Owner: Nobody > Requestors: mmaslano@redhat.com > Status: new > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=50557 > > > > perl script that uses Crypt::SSLeay is crashing since upgrade to openssl > 1.0. Backtrace head with various debuginfo installed: > > #0 freelist_insert (ctx=0xa18c140, for_read=1, sz=34120, mem=0xa0d6a80) > at s3_both.c:645 > #1 0x0072f305 in ssl3_release_read_buffer (s=0xa0c4780) at s3_both.c:762 > #2 0x0072b60c in ssl3_free (s=0xa0c4780) at s3_lib.c:2151 > #3 0x00733d75 in tls1_free (s=0xa0c4780) at t1_lib.c:163 > #4 0x00742041 in SSL_free (s=0xa0c4780) at ssl_lib.c:581 > #5 0x00a6a7c5 in XS_Crypt__SSLeay__Conn_free (my_perl=0x9242008, > cv=0xa002d54) > at SSLeay.c:521 > > It looks like the problem is with free'ing. SSL_CTX_free() is called as > first and then SSL_free(). IMHO these calls should be switched and > SSL_free should be called as first. I'm not familiar with XS module, > therefore I'd rather wait for your opinion.

I've no firm idea myself but your hypothesis sounds plausible. You can edit the source yourself and see if it fixes the problem. If it does, I'll push a new release out. There's a number of other things that need to be taken care of at the same time. Later, David Show quoted text

> Whole bug report could be found here: > https://bugzilla.redhat.com/show_bug.cgi?id=521342 >

-- it's an anthem in a vacuum on a hyperstation, day dreaming days in a daydream nation

Fri Oct 16 17:53:57 2009 The RT System itself - Status changed from 'new' to 'open'

Mon Oct 19 02:15:04 2009 mmaslano [...] redhat.com - Correspondence added

From:

mmaslano [...] redhat.com

Show quoted text

> > It looks like the problem is with free'ing. SSL_CTX_free() is called as > > first and then SSL_free(). IMHO these calls should be switched and > > SSL_free should be called as first. I'm not familiar with XS module, > > therefore I'd rather wait for your opinion.

> > I've no firm idea myself but your hypothesis sounds plausible. You can > edit the source yourself and see if it fixes the problem. If it does, > I'll push a new release out. > > There's a number of other things that need to be taken care of at the > same time. > > Later, > David > >

> > Whole bug report could be found here: > > https://bugzilla.redhat.com/show_bug.cgi?id=521342 > >

> >

I tried switch these calls and it worked. I'd rather take fix from you because I'm not sure what else could be broken by this change. Maintainer of openssl informed me that there was bug also in openssl package which will be fixed in next release, but the fix should be done in both packages.

Mon Nov 23 14:50:33 2009 kmx [...] cpan.org - Correspondence added

Hi, the OpenSSL 1.0.0 issue is related to a version detection routine in Makefile.pl that is not ready for version numbers starting with 1. The current code detects "undef" version and based on that we define: #define CRYPT_SSLEAY_free free instead of #define CRYPT_SSLEAY_free OPENSSL_free Here is a simple patch for Makefile.PL that solves it: my $version; my $type; while (<VERSION_FILE>) { - if (/^#define\s+$version_match\s+0x0+(\d\d\d)/) { + if (/^#define\s+$version_match\s+0x(\d\d\d\d\d)/) { $version = $1; - $version =~ s/(\d)0(\d)/$1$2/; + $version =~ s/^(\d)0(\d)0(\d)/$1$2$3/; $type = ($version > 92) ? "OpenSSL" : "SSLeay"; - $version = join('.', split(//, "0$version")); + $version = join('.', split(//, $version)); last; } } -- kmx

Thu Nov 26 10:55:35 2009 mmaslano [...] redhat.com - Correspondence added

On Mon Nov 23 14:50:33 2009, KMX wrote: Show quoted text

> Hi, > > the OpenSSL 1.0.0 issue is related to a version detection routine in > Makefile.pl that is not ready for version numbers starting with 1.

Hello, I believe this is different issue because the crash was fixed in OpenSSL upstream. Anyway this patch looks useful. Regards, Marcela

Tue Dec 01 04:17:53 2009 kmx [...] cpan.org - Correspondence added

Hi, Show quoted text

> I believe this is different issue because the crash was fixed in OpenSSL > upstream. > Anyway this patch looks useful.

OK, do you think it is worth starting a new RT? For us (Win32/strawberry perl project) this patch is quite important as we are not able to use Crypt::SSLeay+openssl-1.0.0 without it. Thanks for feedback. -- kmx

Mon Aug 09 13:38:03 2010 nanis [...] runu.moc.invalid - Correspondence added

Hello: I recently took on the task of clearing up the RT queue for Crypt-SSLeay. I am trying to understand what issues still need fixing and familiarize myself with the details of the code. Looking at https://bugzilla.redhat.com/show_bug.cgi?id=521342 I am wondering if the fix you propose below is still necessary. If so, could you give me an idea where it is supposed to go in SSLeay.xs as SSLeay.c is generated from that file. -- Sinan On Fri Oct 16 07:12:15 2009, mmaslano@redhat.com wrote: Show quoted text

> perl script that uses Crypt::SSLeay is crashing since upgrade to openssl > 1.0. Backtrace head with various debuginfo installed: > > #0 freelist_insert (ctx=0xa18c140, for_read=1, sz=34120, mem=0xa0d6a80) > at s3_both.c:645 > #1 0x0072f305 in ssl3_release_read_buffer (s=0xa0c4780) at s3_both.c:762 > #2 0x0072b60c in ssl3_free (s=0xa0c4780) at s3_lib.c:2151 > #3 0x00733d75 in tls1_free (s=0xa0c4780) at t1_lib.c:163 > #4 0x00742041 in SSL_free (s=0xa0c4780) at ssl_lib.c:581 > #5 0x00a6a7c5 in XS_Crypt__SSLeay__Conn_free (my_perl=0x9242008, > cv=0xa002d54) > at SSLeay.c:521 > > It looks like the problem is with free'ing. SSL_CTX_free() is called as > first and then SSL_free(). IMHO these calls should be switched and > SSL_free should be called as first. I'm not familiar with XS module, > therefore I'd rather wait for your opinion. > > Whole bug report could be found here: > https://bugzilla.redhat.com/show_bug.cgi?id=521342

Thu Aug 12 01:01:10 2010 http://mmaslano.livejournal.com/ - Correspondence added

On Mon Aug 09 13:38:03 2010, NANIS wrote: Show quoted text

> Hello: > > I recently took on the task of clearing up the RT queue for > Crypt-SSLeay. I am trying to understand what issues still need fixing > and familiarize myself with the details of the code. > > Looking at https://bugzilla.redhat.com/show_bug.cgi?id=521342 I am > wondering if the fix you propose below is still necessary. > > If so, could you give me an idea where it is supposed to go in SSLeay.xs > as SSLeay.c is generated from that file. > > -- Sinan

Hello, I can't reproduce it anymore with v0.57. Maybe changes in openssl clean this problem. Anyway patch for Makefile which was mentioned above is still needed. Regards, Marcela

Thu Aug 12 11:40:46 2010 nanis [...] runu.moc.invalid - Correspondence added

On Thu Aug 12 01:01:10 2010, http://mmaslano.livejournal.com/ wrote: Show quoted text

> On Mon Aug 09 13:38:03 2010, NANIS wrote:

Show quoted text

> > Looking at https://bugzilla.redhat.com/show_bug.cgi?id=521342 I am > > wondering if the fix you propose below is still necessary. > >

... Show quoted text

> Hello, I can't reproduce it anymore with v0.57. Maybe changes in openssl > clean this problem. Anyway patch for Makefile which was mentioned

above is Show quoted text

> still needed.

The version detection stuff should be fixed in 0.57_04. Could you please try that. I am going to close this ticket for now. If you get more information, please re-open it. -- Sinan

Thu Aug 12 11:40:47 2010 nanis [...] runu.moc.invalid - Status changed from 'open' to 'resolved'