NetBSD Problem Report #50918

From www@NetBSD.org  Wed Mar  9 10:47:50 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id AE12E7ABE6
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  9 Mar 2016 10:47:50 +0000 (UTC)
Message-Id: <20160309104749.D6B367ABF7@mollari.NetBSD.org>
Date: Wed,  9 Mar 2016 10:47:49 +0000 (UTC)
From: dcb314@hotmail.com
Reply-To: dcb314@hotmail.com
To: gnats-bugs@NetBSD.org
Subject: src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c:1817]: (error) Memory leak: new
X-Send-Pr-Version: www-1.0

>Number:         50918
>Category:       security
>Synopsis:       src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c:1817]: (error) Memory leak: new
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    security-officer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Mar 09 10:50:00 +0000 2016
>Closed-Date:    Wed Mar 09 16:32:35 +0000 2016
>Last-Modified:  Fri Apr 15 07:55:00 +0000 2016
>Originator:     David Binderman
>Release:        cvs dated 20160308
>Organization:
>Environment:
>Description:
Source code is

            new->login = vdup(xauth_rmconf->login);
            if (new->login == NULL) {
                plog(LLV_ERROR, LOCATION, NULL,
                    "xauth_rmconf_dup: malloc failed (login)\n");
                return NULL;
            }

Looks like a missing call to free inside the if to me.

Also, here are some other minor errors in the same file

[src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c:1633]: (style) Variable 'data' is assigned a value that is never used.
[src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c:1653]: (style) Variable 'xst' is assigned a value that is never used.
[src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c:658]: (style) Variable 'replies' is assigned a value that is never used.

>How-To-Repeat:

>Fix:

>Release-Note:

>Audit-Trail:
From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: security/50918:src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c:1817]:
 (error) Memory leak: new (fwd)
Date: Wed, 9 Mar 2016 19:45:13 +0800 (PHT)

 A quick check shows that there are actually two places where we might 
 need to free(new), and in the second instance we also need to free any 
 successful copy of the new->login member...

 (Starting at line 1812 of isakmp_xauth.c rev 1.27)

   		if (xauth_rmconf->login != NULL) {
   			new->login = vdup(xauth_rmconf->login);
   			if (new->login == NULL) {
   				plog(LLV_ERROR, LOCATION, NULL,
   				    "xauth_rmconf_dup: malloc failed (login)\n");
 +				racoon_free(new);
   				return NULL;
   			}
   		}
   		if (xauth_rmconf->pass != NULL) {
   			new->pass = vdup(xauth_rmconf->pass);
   			if (new->pass == NULL) {
   				plog(LLV_ERROR, LOCATION, NULL,
   				    "xauth_rmconf_dup: malloc failed (password)\n");
 +				if (new->login != NULL)
 +					vfree(new->login);
 +				racoon_free(new);
   				return NULL;
   			}
   		}



 +------------------+--------------------------+------------------------+
 | Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:      |
 | (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
 | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
 +------------------+--------------------------+------------------------+

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/50918 CVS commit: src/crypto/dist/ipsec-tools/src/racoon
Date: Wed, 9 Mar 2016 10:58:25 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Wed Mar  9 15:58:25 UTC 2016

 Modified Files:
 	src/crypto/dist/ipsec-tools/src/racoon: isakmp_xauth.c

 Log Message:
 PR/50918: David Binderman: Fix memory leak


 To generate a diff of this commit:
 cvs rdiff -u -r1.27 -r1.28 \
     src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Wed, 09 Mar 2016 16:32:35 +0000
State-Changed-Why:
Christos fixed it, thanks.


From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/50918 CVS commit: [netbsd-7] src/crypto/dist/ipsec-tools/src/racoon
Date: Fri, 15 Apr 2016 07:52:15 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Fri Apr 15 07:52:15 UTC 2016

 Modified Files:
 	src/crypto/dist/ipsec-tools/src/racoon [netbsd-7]: isakmp.c
 	    isakmp_cfg.c isakmp_ident.c isakmp_xauth.c

 Log Message:
 Pull up following revision(s) (requested by phx in ticket #1145):
 	crypto/dist/ipsec-tools/src/racoon/isakmp_cfg.c: revision 1.26
 	crypto/dist/ipsec-tools/src/racoon/isakmp.c: revision 1.75
 	crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c: revision 1.28
 	crypto/dist/ipsec-tools/src/racoon/isakmp_ident.c: revision 1.14
 PR/50918: David Binderman: Fix memory leak
 --
 From Frank Wille:
 Request "IKE mode config" in "rsasig" (certificates on both sides only)
 authentication mode, if "mode_cfg" is configured to "on".
 Tested with a Lancom router, using the following configuration:
 path include "/etc/racoon";
 path certificate "/etc/racoon/certs";
 path script "/etc/racoon/scripts";
 remote "wpsd"
 {
     remote_address 1.2.3.4;
     exchange_mode main,base;
     my_identifier asn1dn;
     certificate_type x509 "vpnclient15.crt" "vpnclient15.key";
     ca_type x509 "ca.crt";
     mode_cfg on;
     dpd_delay 20;
     nat_traversal on;
     lifetime time 8 hour;
     script "phase1-up.sh" phase1_up;
     script "phase1-down.sh" phase1_down;
     proposal {
         encryption_algorithm aes;
         hash_algorithm md5;
         authentication_method rsasig;
         dh_group 2;
     }
     proposal_check obey;
 }
 sainfo anonymous
 {
     pfs_group 2;
     lifetime time 8 hour;
     encryption_algorithm aes;
     authentication_algorithm hmac_md5;
     compression_algorithm deflate;
 }


 To generate a diff of this commit:
 cvs rdiff -u -r1.74 -r1.74.20.1 \
     src/crypto/dist/ipsec-tools/src/racoon/isakmp.c
 cvs rdiff -u -r1.25 -r1.25.8.1 \
     src/crypto/dist/ipsec-tools/src/racoon/isakmp_cfg.c
 cvs rdiff -u -r1.13 -r1.13.28.1 \
     src/crypto/dist/ipsec-tools/src/racoon/isakmp_ident.c
 cvs rdiff -u -r1.27 -r1.27.4.1 \
     src/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search