[Date Prev][Date Next] [Chronological] [Thread] [Top]

frivolous use of strncmp in ldappasswd.c



The C library's str* functions are known to be the root cause of many
buffer overflow problems.  strcmp() is not dangerous the same way that
sprintf().  The following code has a frivolous use of strncmp that does
more harm than good.

clients/tools/ldappasswd.c:520

	if( want_newpw && newpw == NULL ) {
		/* prompt for new password */
		char *cknewpw;
		newpw = strdup(getpassphrase("New password: "));
		cknewpw = getpassphrase("Re-enter new password: ");

		if( newpw== NULL || cknewpw == NULL ||
			strncmp( newpw, cknewpw, strlen(newpw) ))
		{
			fprintf( stderr, "passwords do not match\n" );
			return EXIT_FAILURE;
		}
	}

There is no good reason for the use of strncmp().  The use of strdup()
guarantees that both strings are terminated.  strcmp() should be used. 
Why?  If newpw is set to `mynewpas' (one `s') and cknewpw is set to
`mynewpass' (two of 'em), a user may think that the new password is
being set to `mynewpass' when it is really being set to the mistyped
`mynewpas' (one `s').  If strcmp() were used, different length strings
would be caught.

This same problem exists at line 513 as well:

		strncmp( oldpw, ckoldpw, strlen(oldpw) ))

And at servers/slapd/tools/slappasswd.c:100

		if( strncmp( newpw, cknewpw, strlen(newpw) )) {

Patch against CVS is included.

Mike

Index: clients/tools/ldappasswd.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/clients/tools/ldappasswd.c,v
retrieving revision 1.32.4.20
diff -c -r1.32.4.20 ldappasswd.c
*** clients/tools/ldappasswd.c	2001/08/28 17:14:48	1.32.4.20
--- clients/tools/ldappasswd.c	2001/12/17 12:46:31
***************
*** 510,516 ****
  		ckoldpw = getpassphrase("Re-enter old password: ");
  
  		if( oldpw== NULL || ckoldpw == NULL ||
! 			strncmp( oldpw, ckoldpw, strlen(oldpw) ))
  		{
  			fprintf( stderr, "passwords do not match\n" );
  			return EXIT_FAILURE;
--- 510,516 ----
  		ckoldpw = getpassphrase("Re-enter old password: ");
  
  		if( oldpw== NULL || ckoldpw == NULL ||
! 			strcmp( oldpw, ckoldpw ))
  		{
  			fprintf( stderr, "passwords do not match\n" );
  			return EXIT_FAILURE;
***************
*** 524,530 ****
  		cknewpw = getpassphrase("Re-enter new password: ");
  
  		if( newpw== NULL || cknewpw == NULL ||
! 			strncmp( newpw, cknewpw, strlen(newpw) ))
  		{
  			fprintf( stderr, "passwords do not match\n" );
  			return EXIT_FAILURE;
--- 524,530 ----
  		cknewpw = getpassphrase("Re-enter new password: ");
  
  		if( newpw== NULL || cknewpw == NULL ||
! 			strcmp( newpw, cknewpw ))
  		{
  			fprintf( stderr, "passwords do not match\n" );
  			return EXIT_FAILURE;
Index: servers/slapd/tools/slappasswd.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/tools/slappasswd.c,v
retrieving revision 1.2.2.7
diff -c -r1.2.2.7 slappasswd.c
*** servers/slapd/tools/slappasswd.c	2001/06/14 00:05:16	1.2.2.7
--- servers/slapd/tools/slappasswd.c	2001/12/17 12:46:31
***************
*** 97,103 ****
  		newpw = strdup(getpassphrase("New password: "));
  		cknewpw = getpassphrase("Re-enter new password: ");
  
! 		if( strncmp( newpw, cknewpw, strlen(newpw) )) {
  			fprintf( stderr, "Password values do not match\n" );
  			return EXIT_FAILURE;
  		}
--- 97,103 ----
  		newpw = strdup(getpassphrase("New password: "));
  		cknewpw = getpassphrase("Re-enter new password: ");
  
! 		if( strcmp( newpw, cknewpw )) {
  			fprintf( stderr, "Password values do not match\n" );
  			return EXIT_FAILURE;
  		}