Re: Possible memory leak in service_ssl.c for snort-2.9.7.x and Snort++?

["Costas Kleopa (ckleopa)" <ckleopa () cisco com>]

I had engineering looking into this and they verified that there's no memory leak there.
Your fix would actually cause double free's.  That ss variable is coming from way above in that code.

When it's created, it's attached to the session with the call to ssl_service_mod.api->data_add(...).  It's typically 
just deleted when the session is closed out and freed up.

Thanks
Costas


On Mar 19, 2015, at 5:47 PM, Bill Parker <wp02855 () gmail com<mailto:wp02855 () gmail com>> wrote:

Hello All,

    In reviewing code in '/src/dynamic-preprocessors/appid/service_plugins'
file 'service_ssl.c', I found a call to malloc() while checked for a return
value of NULL, a return SERVICE_ENOMEM; executed immediately upon failure,
but variable 'ss' was previously assigned memory by a call to calloc() at
line 629 in function 'MakeRNAServiceValidationPrototype', should it not
be released prior to the return SERVICE_ENOMEM;?  The patch file below
corrects this issue:

--- service_ssl.c.orig  2015-03-19 10:09:57.358608756 -0700
+++ service_ssl.c       2015-03-19 10:16:01.295863310 -0700
@@ -762,8 +762,10 @@
                         certs_rec = (ServiceSSLV3CertsRecord *)data;
                         ss->certs_len = ntoh3(certs_rec->certs_len);
                         ss->certs_data = malloc(ss->certs_len);
-                        if (!ss->certs_data)
+                        if (!ss->certs_data) {
+                            free(ss);
                             return SERVICE_ENOMEM;
+                       }
                         if ((size - sizeof(ServiceSSLV3CertsRecord)) < ss->certs_len)
                         {
                             /* Will have to get more next time around. */

I am attaching the patch file to this possible bug report.

Bill Parker (wp02855 at gmail dot com)
<service_ssl.c.patch>------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net<mailto:Snort-devel () lists sourceforge net>
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/

_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!