Password Validation Else Statement Firing When It Shouldn't

Question

I am trying to validate a password in C, and one of my else statements is firing automatically whenever the code runs. I run a test to see if a character is a symbol, and if it is add 1 to int symbol using symbol++;, but the problem is that this code is executing regardless of whether the character I am testing is a symbol.

I think this issue has something to do with the structure of my if, else statements, and I have tried several combinations, but something is wrong that is throwing the program off I've used else if but that didn't help. This seems like it should be really obvious, but I can't seem to figure out what's wrong.

char password[30];
int lower, upper, number, symbol, i;
lower = upper = number = symbol = 0;

printf("Enter your password: ");
scanf("%s", &password);

int len = strlen(password);

for (i = 0; i <= len; i++) {

    if (isalpha(password[i])){

        if (isupper(password[i])){
            upper++;
        }

        else{
            lower++;
        }
    }

    if (isdigit(password[i])){
        number++;
    }

    else{
        symbol++;
    }
}

if (upper >= 1 && lower >= 1 && number >= 1 && symbol >= 1 && len >=6){

    printf("Your password is good!");

}

if (upper < 1){

    printf("You need an uppercase letter \n");

}

if (lower < 1){

    printf("You need a lowercase letter \n");

}

if (number < 1){

    printf("You need a number \n");

}

if (symbol < 1){

    printf("You need a symbol \n");

}

if (len < 6){

    printf("Your password must be at least 6 characters \n");

}

Answer 1

In your code, change

for (i = 0; i <= len; i++) 

to

for (i = 0; i < len; i++) 

as, C arrays have 0 based index. Otherwise, you may be overrunning allocated memory which in turn invokes undefined behaviour.

Note: Even if you don't overrun memory (as you have a compile time allocated array and the input maybe less than the actual array size), you'll end up comparing the terminating nul, which probably you don't want.

Then , the isdigit() check should not a standalone if (as per your logic), it should be an else if with isalpha().

That said,

 scanf("%s", &password);

should be

 scanf("%29s", &password);

to avoid any possible risk of buffer overflow.

Answer 2

The line

symbol++;

will execute when an alpha is entered.

To prevent this, insert else before the isdigit test.

else if (isdigit(password[i])) {

Also the loop is incorrect as others point out. It should be

for (i = 0; i < len; i++) {

Answer 3

You have code that looks like this:

if (cond1){}
if (cond2){}
else {}

In this case, execution of the else-block is independent of the cond1-block, as stated by the C11 draft standard, Section 6.8.4.1 $3: An else is associated with the lexically nearest preceding if that is allowed by the syntax.

You can change the structure of the code to:

if (cond1){}
else if (cond2){}
else {}