pgcrypto: fix memory leak in openssl.c

Started by Marko Kreenover 20 years ago5 messagespatches
Jump to latest
#1Marko Kreen
markokr@gmail.com

pgcrypto crypt()/md5 and hmac() leak memory when compiled against
OpenSSL as openssl.c digest ->reset will do two DigestInit calls
against a context. This happened to work with OpenSSL 0.9.6
but not with 0.9.7+.

Reason for the messy code was that I tried to avoid creating
wrapper structure to transport algorithm info and tried to use
OpenSSL context for it. The fix is to create wrapper structure.

It also uses newer digest API to avoid memory allocations
on reset with newer OpenSSLs.

Attached are one patch for 7.3, 7.4, 8.0 branches and another
for 8.1 and HEAD.

Thanks to Daniel Blaisdell for reporting it.

--
marko

Attachments:

fix.73.74.80.difftext/x-patch; charset=us-ascii; name=fix.73.74.80.diffDownload+80-44
fix.81.82.difftext/x-patch; charset=us-ascii; name=fix.81.82.diffDownload+98-91
#2Neil Conway
neilc@samurai.com
In reply to: Marko Kreen (#1)
Re: pgcrypto: fix memory leak in openssl.c

On Sat, 2006-02-18 at 02:23 +0200, Marko Kreen wrote:

Attached are one patch for 7.3, 7.4, 8.0 branches and another
for 8.1 and HEAD.

Thanks, patches applied to the appropriate branches.

-Neil

#3Marko Kreen
markokr@gmail.com
In reply to: Marko Kreen (#1)
Re: pgcrypto: fix memory leak in openssl.c

On 2/18/06, Marko Kreen <markokr@gmail.com> wrote:

pgcrypto crypt()/md5 and hmac() leak memory when compiled against
OpenSSL as openssl.c digest ->reset will do two DigestInit calls
against a context. This happened to work with OpenSSL 0.9.6
but not with 0.9.7+.

Ugh, seems I read the old code slightly wrong. The leak happens
also with regular digest(), although it will leak only 1 context
instance, not the 1000+ as the crypt-md5 does. And on 8.1 there
is pgp_sym_encrypt that also does lots of resets on one context,
like crypt-md5. In addition it does regular digest() in several
places. So if compiled against OpenSSL, its leaking everywhere.

The positive side is that only 8.1 has openssl autoconfiguration,
older versions default to builtin algorithms that can be changed
only by editing Makefile, thus most packages are hopefully safe.

--
marko

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Marko Kreen (#3)
Re: pgcrypto: fix memory leak in openssl.c

"Marko Kreen" <markokr@gmail.com> writes:

On 2/18/06, Marko Kreen <markokr@gmail.com> wrote:

pgcrypto crypt()/md5 and hmac() leak memory when compiled against
OpenSSL as openssl.c digest ->reset will do two DigestInit calls
against a context. This happened to work with OpenSSL 0.9.6
but not with 0.9.7+.

Ugh, seems I read the old code slightly wrong. The leak happens
also with regular digest(), although it will leak only 1 context
instance, not the 1000+ as the crypt-md5 does.

I'm confused --- does this mean that the patch you sent recently
needs further work?

regards, tom lane

#5Marko Kreen
markokr@gmail.com
In reply to: Tom Lane (#4)
Re: pgcrypto: fix memory leak in openssl.c

On 2/20/06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Marko Kreen" <markokr@gmail.com> writes:

On 2/18/06, Marko Kreen <markokr@gmail.com> wrote:

pgcrypto crypt()/md5 and hmac() leak memory when compiled against
OpenSSL as openssl.c digest ->reset will do two DigestInit calls
against a context. This happened to work with OpenSSL 0.9.6
but not with 0.9.7+.

Ugh, seems I read the old code slightly wrong. The leak happens
also with regular digest(), although it will leak only 1 context
instance, not the 1000+ as the crypt-md5 does.

I'm confused --- does this mean that the patch you sent recently
needs further work?

No, it's fine. As I did not 'fix' old code but replaced it.

It's just that I gave wrong answer to the question 'who is affected?'

--
marko