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

Re: frivolous use of strncmp in ldappasswd.c

Sounds like a bug, please report using the Issue Tracking System
<http://www.openldap.org/its/>.  Thanks, Kurt

At 04:52 AM 2001-12-17, Mike  Gerdts wrote:
>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.
>        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.